Code review comment for lp:~hazmat/txzookeeper/distributed-queue

> The design of SerializedQueue looks pretty nice. Thank you!
>
> A few additional (last?) comments below:
>
> [20]
>
> In SerializedQueue:
>
> if name.endswith(suffix):
> children[:] = []
>
> Putting some debugging information, I see that this logic is actually used,
> but I'm a bit confused regarding when it's necessary. The tests which reach
> this logic also make me a bit worried:
> test_staged_multiproducer_multiconsumer, which theoretically should be a case
> which should never see items being processed due to the lock.

the lock is only held when fetching an item. ie. the lock guards access to the queue items. However the consumer might still be processing the item, and until it does the processing node isn't released. it might be a better semantic (less contention) for serialization if the lock was held for the duration of the item processing. i'll make that change.

>
> [21]
>
> Something that occurred to me while I was reading the code is that the line:
>
> request.processing_children = False
>
> May still not get executed on certain exists from the logic in the item
> processing. Should we guarantee that it will necessarily be reset on any
> exit, so that we never get an unusable object (with an inconsistent state)?
>

is there a scenario where this would be the case? afaics all the failure mode exists are covered by error handlers that will set this flag to false.