Forum OpenACS Improvement Proposals (TIPs): Replace request processor reload code with ns_eval

The purpose of this TIP is to simplify and make more reliable the reload tcl/query file code in the APM.

Right now the files are marked for reload in an nsv, and when a request is made the file is reloaded in that one interpreter. Eventually every request thread should have the reloaded library. This only works in request threads. It won't reload a tcl library in a scheduled proc thread. This requires a server restart. (or ns_eval which is what I am proposing to use here.)

The ns_eval procedure in AOLserver will run code in all interpreters included a scheduled procedure thread. This simplies the reloading and simplifies the request processor a tiny bit. ns_eval can be used to reload tcl libraries and query files. The following two commands can be used.

ns_eval {source [acs_root_dir]/packages/${package_key}/tcl/${tcl-library-file}}

ns_eval {db_qd_load_query_file [acs_root_dir]/packages/${package_key}/tcl/${query-file}}

The code change would need to be made in the APM to directly reload the files instead of marking them for reload and relying on the request processor magic.

Posted by Don Baccus on
Yes, this would be a good change. You've tested it, yes?

IIRC there was no way to do this back in the day when this code was first written for later versions of ACS 3.x, and that's why the RP hack was developed.

It will not only simplify but slightly speed up the RP, too.

Posted by Dave Bauer on

I have so far testing ns_eval from the ds/shell and from there everything works as expected. I am working on making that changes to the APM.

Yes ns_eval is definitely new, and perhaps there was some question in the past of its reliability. Now that we have problems with out custom reload code, it makes sense to replace it with ns_eval, which, according to my informal testing so far, is much more reliable.

I have been using ns_eval for a year for one client to reload code on a server that is running with performance_mode_p turned on. This is great for fixing little problems in a produciton site without having to take the server down.

Posted by Don Baccus on
"Yes ns_eval is definitely new, and perhaps there was some question in the past of its reliability."

Naw, it's more like the thread model for aolserver changed and none of us paid enough attention, well, not me at least - I remember reading about ns_eval but didn't make the connection with watching tcl lib files.

Go for it, and hey, while you're at it, make the APM use the template form
builder :) :)

Why would (or should) this change require a TIP? Sounds like a code cleanup issue to me, plus adding a bit of nice new functionality (the reloading of new procs into non-request threads). If the change works right it won't change any existing functionality at all.
Posted by Dave Bauer on
I never implemented this but it would make life easier. Is this a good idea, or is there a better solution 10 years later?
Posted by Gustaf Neumann on
It is still a good idea, although one has to differentiate between "run a command in all threads" vs. "update the blueprint". If the intention is to update e.g. a variable in a tcl-namespace, the first one can be very lightweight, while in cases, where the blueprint is large updating it can be quite costly (since after invalidation of the blueprint of a thread, the whole blueprint has to be re-evaluated (takes in 0.7 secs, will be worse on e.g. dotlrn installations), which might lead to "random", unexpected delays. Furthermore, just using ns_eval will not be sufficient, if we want to keep features like e.g. "watching" of files, etc.

There is a small machinery [1] which resolves the issue of the e.g. updating schedule threads by allowing to send commands to other threads in a more generic way than in the request processor. This approach updates the after-request callbacks to issue update commands in cleanup steps. Therefore it affects all threads as long these call the cleanup callbacks (which is a good idea as well to cleanup e.g. ns_sets, etc.) This requires some work. The functionality can be made independent from xoctl-core and can be added to the core.

I think, we should add a "refresh blueprint reform" the to the wish-list of the release after 5.9.1.