[FIXED] JetStream: Some stream advisories missing

[kozlovic]

The "deleted" advisory was missing because the stream's send loop
was closed before the advisory was pushed to the queue to be sent.

Added tests, both for single and clustered mode to test all stream
advisories.

Resolves #2886

Signed-off-by: Ivan Kozlovic ivan@synadia.com

[kozlovic]

@derekcollison I have created this PR with a test that checks for all stream advisory. The "deleted" advisory missing was due to a close of the channel before we scheduled the advisory (even this current fix may not be 100% reliable, since I am not certain that Go's select{} with case <-channel would consume always in the order they are listed. We know there will be a message pushed in the queue, but the close of the channel may be picked-up first?

Anyway, the other issue is as described in one review-comment: not sure how to make the advisory be sent in case of the restore of the stream in clustered mode.

[derekcollison]

ok so next steps for this one are what?

[kozlovic]

ok so next steps for this one are what?

That you figure out how to have the server send the advisory, in cluster mode, when doing a stream recover. As I pointed out, the code suppress the advisory because the server is leader, clustered, but sa is nil.

[derekcollison]

ok and the delete advisory is fixed to your satisfaction?

Should I simply extend this PR or create new one?

[kozlovic]

You should extend, since there is the new test already (which obviously fails because of the missing "created" advisory on stream restore).
As for the delete, moving the close of channel seem to do the trick, but not sure if this will work 100%.

[kozlovic]

LGTM. I can't "approve" this PR since I am considered the author (GitHub won't let me select "Approve"). But I can confirm that the changes you made allow the update advisory to be seen in clustered/R3 and the create advisory after a snapshot restore.

[ripienaar]

LGTM