-
Notifications
You must be signed in to change notification settings - Fork 937
Ignore primary lease loss in maybeGarbageCollectMultiClientState #2585
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
Conversation
This ignores the error if the primary lease is somehow lost between updateClientMetadataAndTryBecomePrimary() and maybeGarbageCollectMultiClientState(). There is no special recovery logic needed as the next primary will run the periodic GC. Fixes #2555
@@ -489,7 +488,12 @@ export class IndexedDbPersistence implements Persistence { | |||
).next(() => inactive); | |||
}); | |||
} | |||
); | |||
).catch(() => { | |||
// Ignore primary lease violations or any other type of error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's worth adding why it's OK to ignore all the errors here.
Something along the line where you put in PR description: There is no special recovery logic needed as the next primary will run the periodic GC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but maybe explain why it is OK to ignore errors in the comments as well?
@wilhuff for approval |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This ignores the error if the primary lease is somehow lost between updateClientMetadataAndTryBecomePrimary() and maybeGarbageCollectMultiClientState(). There is no special recovery logic needed as the next primary will run the periodic GC.
Fixes #2555