VCommitter
commented
7 years ago I don't think you mean adding a status to Vsa_VEvaluator.h#L97 and cover all of it's usage in the class hierarchy. m_iStatus lead me down this false path.
I propose a flag such as explicitlySuspended in VEvaluatorPool. The callback acts appropriately and we extend suspend() and resume() to set and unset explictlySuspended (respectively).
The only known unknown is how should the VEvaluatorPool behave if it gets a resume() while suspended due to a hardRestart()? Should it perform the BaseClass::resume() or just unset explicitlySuspended and wait for the callback to resume()?
Right now there is another related bug: if the VEvaluatorPool is suspended due to stacked hardRestart() it won't actually resume() until you send it an explicit suspend(). resume() fails with Error: Operation Failed until the explicit suspend() is sent. I haven't dug into this yet because I'm hoping fixing the first bug obviates the second.
@c-kuhlman how is poolTester looking?
MichaelJCaruso
c-kuhlman
Leslie asked me to take a look at a problem with hard restarts of release 8.1 pools. Specifically, if a succession of hard restart messages are sent to a running pool and those messages arrive faster than the pool can actually be restarted, the pool is inadvertently left in a suspended state instead of being 'restarted' as expected. I have confirmed this (mis)behavior and have tracked it to a fairly straightforward logic bug in a change to the implementation of hard restart in release 8.1.
In release 8.1, hard restart was changed to wrap its restart processing in a suspend/resume pair. While suspending a running pool is atomic and instantaneous, restarting one is not. To deal with that, a callback is set _(Vsa_VEvaluatorPool.cpp#736)_ that performs the needed restart wrap-up and resumes the pool after the restart processing has finished. _There is exactly one slot for remembering that callback (Vsa_VEvaluatorPool.cpp#794)._ Other than the routine embedded in the callback, there is no record of the fact that a resume must be run after the restart has completed. Hard restart messages received before the currently running hard restart has finished destroy that information, resulting in the observed bug.
Hard restart messages received by a pool that is already processing a hard restart follow a different path. Noting that the pool is already suspended, they do not attempt to wrap their restart processing in a suspend/resume pair. Instead, after initiating a restart, they register a callback _(Vsa_VEvaluatorPool.cpp#733)_ that performs the needed restart wrap-up and leaves the pool in the suspended state in which it was found. Note that there is only one slot for remembering that callback and that the callback that would have done the required resume has just been obliterated.
It seems to me that the crux of the problem is an attempt to encode state information in the opaque machinery of which callback must be run instead of adding a new explicit state that unambiguously identifies the internal suspension being performed. Shouldn't be hard - glad to talk about it further.