move metadata / event pushing in HTTPSpooler into doStopLog#850
move metadata / event pushing in HTTPSpooler into doStopLog#850barentine wants to merge 1 commit into
Conversation
|
note that this also avoids the events/rule pushing race condition I ran into without any clusterpzfdatasource special casing |
|
Unfortunately this might not work as intended as the existence of Hard to be 100% certain if this will cause any issues, but there is a small chance that it will give rise to yet another race condition down the track. It might be safer to leave writing these files where it is and add a new |
|
ah, I see your point. I do kind of think this is still the right place to push the events/metadata though, and wonder if it should just be combined with a spool_complete signal for the pushers to connect? |
|
making the pushers aware of the spooler completion (rather than relying on file existence checks) is on the TODO list, and would be nice, but will only ever work for pushers which are in-process - and not for pushers which are running elsewhere - e.g. triggered from ClusterUI, or from the ruleserver, e.g., with a rule that monitors directories for changes (If you notice the rule structure, |
|
I guess I don't see how we're answering anything other than in-process pushers right now? A non in-process pusher e.g. on clusterUI will still check for the events file --edit-- sorry, this was completely missing your last point. I'm there now! |
|
And a non in-process pusher will be fine as long as the events and/or
metadata files land after the frames. Changing that order will mess up non
in process pushers.
…On Tue, 16 Feb 2021, 14:10 barentine, ***@***.***> wrote:
I guess I don't see how we're answering anything other than in-process
pushers right now? A non in-process pusher e.g. on clusterUI will still
check for the events file
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#850 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEUSWYBEXJMBOUVA6U6NMTTS7HA2HANCNFSM4XRMVYLA>
.
|
|
Closing as issue addressed in #857 in a slightly safer / more paranoid way |
Addresses issue race condition mentioned in #822 .
Proposed changes:
Spooler.StopSpoolrather than after, but importantly puts them after disconnecting from frame source, after protocol.finish() which can still generate events, but beforeself.spoolOn = False.Checklist: