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
Attempting to read from a non-existent database can create many distinct client IDs in the session metrics #202
Comments
I tried updating to master after #206 was merged, but I'm still getting a large number of client IDs. I also tried running the reproduction above, but this time the client IDs ranged from 1 to 50 instead of 1 to 100. |
@sebright The client ids in the range 1 to 50 especially baffles me. Is your test code literally equal to the reproduction code you mentioned above, or could it be that the test code is somehow running parts of it in parallel? |
@sebright Friendly ping. Is this still an issue? |
@olavloite Sorry for the delay. I tried running the reproduction from the first comment again with google-cloud-spanner 1.58.0. This time I didn't see any stats recorded for the metric in_use_sessions, but I saw the same behavior as in #202 (comment) for max_in_use_sessions. There were 50 distinct client IDs (client-1 through client-50). |
@sebright I'm still not able to reproduce this locally using both a real Cloud Spanner instance as well as a local mock server. I've therefore created a small integration test so we can use that to try to figure out where the difference is coming from. The integration test in #353 is based on the example at the beginning of this issue and checks that only one client id is created, and it currently succeeds. Note that:
At the end of the test it verifies that only one client id has been generated and used. Would you mind having a look and see if you can spot where the difference in outcome could be coming from in this test case, compared to the test that you run on your setup? |
Friendly ping @sebright. If you could look at the code that @olavloite wrote, it would help us to repro the issue. |
Since we haven't had updates here in a while, I'm going to close the issue. Please re-open if this issue is still happening. |
Thanks for sharing the integration test. I rebased #353 and tried running it, and I think I understand the difference. My reproduction uses opencensus 0.26.0 and has a dependency on opencensus-impl, and the integration test uses opencensus 0.24.0 without opencensus-impl. The integration test fails when I add these dependencies to
It looks like opencensus-impl 0.26.0 throws an exception when multiple time series are recorded with the same client ID. The test alternates between throwing "Database not found" and "A different time series with the same labels already exists" exceptions in each iteration of the for loop. Here is the stack trace for the time series error:
EDIT: I also noticed that the list SpannerImpl.invalidatedDbClients grew by one element every two iterations, which could cause memory usage to increase. |
Adding the same time series multiple times will cause an error in OpenCensus. A time series could be added multiple times for the same Database client id if a database client was invalidated, for example because if was created for a database that did not exist. Fixes #202
@sebright Thanks for the reproduction case! I'm able to reproduce and solve it for version 0.26 now. See the linked PR for more information, but the TLDR is that OpenCensusImpl 0.24 does not allow the same metric to be registered multiple times, which means that it is not solvable for that version. 0.26 does work as expected with the change in #766. |
Adding the same time series multiple times will cause an error in OpenCensus. A time series could be added multiple times for the same Database client id if a database client was invalidated, for example because if was created for a database that did not exist. Fixes #202
Adding the same time series multiple times will cause an error in OpenCensus. A time series could be added multiple times for the same Database client id if a database client was invalidated, for example because if was created for a database that did not exist. Fixes #202
I noticed that calling
Spanner.getDatabaseClient
with a non-existent database and then trying to read from the database can cause new client IDs to appear in the OpenCensus session metrics. The client IDs for the non-existent database seem problematic because they can cause the size of the metrics to grow.Here is a simplified reproduction:
google-cloud-spanner version: 1.54.0
opencensus-java version: 0.26.0
The metrics exported to Stackdriver contain 100 client IDs. For example, here is
in_use_sessions
:The text was updated successfully, but these errors were encountered: