-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Add filter to GetMinUnfinishedWatermark queryAdd filter tester #35042
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
f3470e8 to
344e4cf
Compare
|
Integration test can pass locally https://screenshot.googleplex.com/6aQBPAW8YT69VTd |
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
922ce28 to
e638136
Compare
|
Run integration test locally based on earlier commit #34883 and it passed. Hence the integration timeout is not caused by this PR. |
|
assign set of reviewers |
|
Assigning reviewers: R: @m-trieu for label java. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
...src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/dao/PartitionMetadataDao.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/cache/AsyncWatermarkCache.java
Show resolved
Hide resolved
...rc/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/cache/AsyncWatermarkCache.java
Show resolved
Hide resolved
...test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java
Show resolved
Hide resolved
1feabc1 to
f7320e1
Compare
...src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/dao/PartitionMetadataDao.java
Outdated
Show resolved
Hide resolved
|
Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment R: @chamikaramj for label java. Available commands:
|
b1761a4 to
c7d4c25
Compare
|
Friendly ping :) |
...src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/dao/PartitionMetadataDao.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/dao/PartitionMetadataDao.java
Outdated
Show resolved
Hide resolved
...test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/cache/NoOpWatermarkCache.java
Show resolved
Hide resolved
...rc/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/cache/AsyncWatermarkCache.java
Outdated
Show resolved
Hide resolved
5a5f4a5 to
5f0a76a
Compare
|
Friendly ping :) |
559ee3c to
3c388b5
Compare
|
Friendly ping :) |
|
It seems GCP IO integration test stucks indefinitely. Retriggered the test update: second try still stuck, retriggering If it timeouts for the third time, consider add class level test timeout for integraton tests that might be affected to debug |
c6deb4c to
26dcc4e
Compare
26dcc4e to
d5bb6b4
Compare
To improve the performance of the query for GetUnfinishedMinWatermark, we can add a filter to the query.
Note that the query run on a cache to refresh every 1s. We can remember the cached value every time and use the value as a filter in the next time query.
Also add class level timeout for all change stream integration tests.
Note that, the CacheLoader's refresh function cannot return null, because that will be deemed invalid, then the cache will not be updated. As the result, the connector will run forever. Hence we still keep the return type optional.