-
Notifications
You must be signed in to change notification settings - Fork 122
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
Aggregate Universe Stats: always store result of querySyncStats #1302
Conversation
Pull Request Test Coverage Report for Build 12904863504Details
💛 - Coveralls |
5b63f13
to
f73aa87
Compare
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.
Concept ACK. Some kind of test case (unit or integration) would be nice.
Our universe stats could run into a case where if in an un-cached state the db query takes longer than the default client timeout (usually 30s) then we'd fail the query and return early. This would cause the next client to also receive an error instead of the cached result, making this codepath stuck in an endless failing query. With this change we parallelize the db query and we always wait for the result in order to store it in cache. We may fail the function earlier, but the result will always be retrieved & persisted.
f73aa87
to
c7f16f6
Compare
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.
Very nice!
With this commit we add a simple test for the asynchronous cache population that was introduced previously. We make a single RPC call to the aggregate stats endpoint with a client timeout that is too small. This leads to an RPC failure, so we check whether the cache was updated in the background.
c7f16f6
to
5aeb9f5
Compare
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.
Nice!
Aggregate Universe Stats: always store result of querySyncStats
Our universe stats could run into a case where if in an un-cached state (like on system start-up) the db query takes longer than the default client timeout (usually 30s) then we'd fail the query and return early. This would cause the next client to also receive an error instead of the cached result, making this codepath stuck in an endless failing query. With this change we parallelize the db query and we always wait for the result in order to store it in cache. We may fail the function earlier, but the result will always be retrieved & persisted. This way, even if the 1st client retrieves an error, the next one will receive the cached (late) result of the previous run.
Closes #1304