jonathansudhakar1 commented on issue #37002: URL: https://github.com/apache/beam/issues/37002#issuecomment-6804260780
@jiangzzhu the four tests from #37003 are still `@Ignore`d on master ([L119](https://github.com/apache/beam/blob/519a74ee36f0d42eeea444949e6ef6ffb5191309/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java#L119), [L160](https://github.com/apache/beam/blob/519a74ee36f0d42eeea444949e6ef6ffb5191309/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java#L160), [L224](https://github.com/apache/beam/blob/519a74ee36f0d42eeea444949e6ef6ffb5191309/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java#L224), [L288](https://github.com/apache/beam/blob/519a74ee36f0d42eeea444949e6ef6ffb5191309/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java#L288)), although this issue was closed b y the stale bot. I ran them on 519a74ee36f with the `@Ignore`s removed. All four fail on outdated expectations rather than a connector bug: - `testUnavailableExceptionRetries`, `testAbortedExceptionRetries` and `testResourceExhaustedRetry`: `readChangeStream()` calls `getDialect()` while the pipeline is being constructed ([SpannerIO.java#L2405](https://github.com/apache/beam/blob/519a74ee36f0d42eeea444949e6ef6ffb5191309/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java#L2405)). The injected sticky error fails that dialect query, and the current client reports the injected status, e.g. `UNAVAILABLE: io.grpc.StatusRuntimeException: UNAVAILABLE - Statement: 'select option_name, option_value from information_schema.database_options where option_name in ('database_dialect', 'default_transaction_isolation')'` (likewise `ABORTED` and `RESOURCE_EXHAUSTED`). The tests still expect `DEADLINE_EXCEEDED`; their comments say `getDialect` used not to bubble up the real message. - `testInvalidRecordReceived` fails with `INTERNAL: There is no result registered for the statement: select option_value from information_schema.change_stream_options …`. It never got the `mockChangeStreamOptions()`, `mockGetPartitionSize(...)` and `mockChangePartitionState(...)` calls that its enabled twin, `testInvalidRecordReceivedWithDefaultSettings`, has. A minimal, test-only fix passes locally: - Expect the injected status in the first three. - Give the fourth the same mocks and expectations as its twin (`PipelineExecutionException`, `Field not found`, and more than 0 `ExecuteSqlRequest`s). - Drop the `@Ignore`s. Running `./gradlew :sdks:java:io:google-cloud-platform:test --tests 'org.apache.beam.sdk.io.gcp.spanner.changestreams.SpannerChangeStreamErrorTest' --rerun` 5 times in a row gave 7/7 passing each time (JDK 21, macOS). One caveat: since the error hits the construction-time dialect query, the three "retry" tests would only check that construction fails with the injected status, not that the change-stream query itself is retried. Making them cover the read path would need the dialect query to succeed while the change-stream query fails, which `setExecuteStreamingSqlExecutionTime` alone can't do. Would a test-only PR with the minimal fix (plus test comments/names that say what they actually check) be welcome, or would you rather rework the retry tests to cover the read path? And should it go under this issue (reopened) or a new one? Related: #21533 is still open for the original `testUnavailableExceptionRetries` failure. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
