#22 doc - must use ns_job_done on persistent jobs
Closed: Fixed Opened by rmeggins.

copied from https://fedorahosted.org/389/ticket/48120

You must use ns_job_done on persistent jobs.

Persistent I/O job callbacks will not be called when the client closes the connection. You cannot depend on being able to call ns_job_done on the persistent job when the connection is closed and the read callback is called. For non-persistent I/O jobs, the read callback will be called when the connection is closed by the client. We need to make sure this behavior is documented - or perhaps it is only a libevent thing?


With #57, because a a job that is PERSIST is always in the ARMED state, ns_job_done will not work on it. The exception is during server shutdown, when ARMED jobs CAN be deleted, but we want that behaviour to occur then.

I think that with #57 this bug is partially invalid.

As well, we have discussed that PERSIST should only be used on ACCEPT jobs, so perhaps we should be enforcing that? Is there another type of PERSIST job that may exist besides socket acceptors?

Replying to [comment:1 firstyear]:

With #57, because a a job that is PERSIST is always in the ARMED state, ns_job_done will not work on it. The exception is during server shutdown, when ARMED jobs CAN be deleted, but we want that behaviour to occur then.

What is the procedure for removing a persistent job? Modifying it to be a non-persistent job?

I think that with #57 this bug is partially invalid.

As well, we have discussed that PERSIST should only be used on ACCEPT jobs,

We have discussed
* accept jobs will usually be PERSIST
* PERSIST jobs will usually be accept
* there may be other use cases for general read/write jobs to be PERSIST that we don't yet use or know about, but we do not want to preclude their use
* even for accept jobs that are PERSIST, there are use cases (such as multi-threaded accepts) where we will want to make the accept job non-persistent while doing multiple accepts

so perhaps we should be enforcing that?

I don't think we should enforce that accept should only be use for PERSIST, or that PERSIST should be only for accept.

Is there another type of PERSIST job that may exist besides socket acceptors?

Not yet for 389.

Well, with the changes I have made recently, a persist job can shut itself down from within it's own fn with ns_job_done. An external call to this won't work, because the job basically flips between RUNNING and ARMED, the exception being when thrpool shutdown is set, then the ARMED check is ignored.

Reality is, without this, there are so many use after frees, crashes and issues. These jobs are difficult, and hard to get right. I am getting much closer now though: but it has limits.

So my advice on stopping a persist job now while running, is that you set a variable, that when the persist cb is called, it then sends itself to ns_job_done. Otherwise, we start to need mutexes etc, which is something we want to avoid.

For actual server shutdown, you would do:

{{{
ns_thrpool_shutdown()
ns_job_done(persist) // done all the persist jobs now
ns_thrpool_destroy()
}}}

Metadata Update from @firstyear:
- Custom field Origin adjusted to Community
- Custom field Review Status adjusted to review
- Issue close_status updated to: None

Metadata Update from @mreynolds:
- Custom field Review Status adjusted to ack (was: review)

commit 4998c860699983814f4b7a2cb7a0a4b12880564a
To ssh://git@pagure.io/nunc-stans.git
7209013..4998c86 master -> master

Metadata Update from @firstyear:
- Issue close_status updated to: Fixed
- Issue status updated to: Closed (was: Open)

Metadata