Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect glob collection subscriptions on reconnect #1011

Merged
merged 5 commits into from
Sep 4, 2024
Merged

Conversation

PapaCharlie
Copy link
Member

The RetryTask did not resubscribe to glob collections after a reconnect. Instead it goes back to map subscriptions. This change honors the glob collection config in the retry task.

The RetryTask did not resubscribe to glob collections after a reconnect. Instead
it goes back to map subscriptions. This change honors the glob collection config
in the retry task.
Copy link
Contributor

@shivamgupta1 shivamgupta1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please update testing details. Recommend to at least test locally for correctness.

Copy link
Contributor

@bohhyang bohhyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch. LGTM. Could deploy indis-canary with this on QEI, find the connected EI observer in the log, use admin portal to let that observer kill all connections, then inspect the log to see the requests sent to the new observer that gets connected.

@PapaCharlie
Copy link
Member Author

Tested locally that this works as expected. After a reconnect, it resubscribes with glob collections

@PapaCharlie PapaCharlie merged commit e2f6fa7 into master Sep 4, 2024
2 checks passed
@PapaCharlie PapaCharlie deleted the pc/retrytask branch September 4, 2024 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants