Skip to content
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

fix(bigtable): Fix deadline exceeded conformance test #9220

Merged
merged 2 commits into from
Jan 24, 2024

Conversation

bhshkh
Copy link
Contributor

@bhshkh bhshkh commented Jan 4, 2024

Issue: TestReadRow_Generic_DeadlineExceeded test:
https://github.com/googleapis/cloud-bigtable-clients-test/blob/af480c9811ae17c6dc01fe4959e7ca0f5c7afa08/tests/readrow_test.go#L81-L82

fails with below errors

=== RUN   TestReadRow_Generic_DeadlineExceeded
[Servr log] 2024/01/04 14:02:17 There is 10s sleep on the server side
    readrow_test.go:118: 
        	Error Trace:	cloud-bigtable-clients-test/tests/readrow_test.go:118
        	Error:      	Not equal: 
        	            	expected: 1
        	            	actual  : 0
        	Test:       	TestReadRow_Generic_DeadlineExceeded
    readrow_test.go:121: 
        	Error Trace:	cloud-bigtable-clients-test/tests/readrow_test.go:121
        	Error:      	Not equal: 
        	            	expected: 4
        	            	actual  : 2
        	Test:       	TestReadRow_Generic_DeadlineExceeded
--- FAIL: TestReadRow_Generic_DeadlineExceeded (2.01s)
FAIL
exit status 1
FAIL	github.com/googleapis/cloud-bigtable-clients-test/tests	2.267s

Cause:

  1. The ReadRow method sends ReadRowsRequest rpc. Since only one row needs to be read, the test validates that row limit should be 1. The library does not set any row limit and thus, the error:
Error:      	Not equal: 
        	expected: 1
        	actual  : 0
  1. The ReadRow method returns the context deadline exceeded error as is. This error is not in GRPC error format.

Fix:

  1. Send row limit in the ReadRowsRequest
  2. Obtain status code from context related errors and create errors in GRPC status format. So, the code from error is Unknown i.e. 2 instead of DeadlineExceeded i.e. 4

After fix:

=== RUN   TestReadRow_Generic_DeadlineExceeded
[Servr log] 2024/01/04 14:04:30 There is 10s sleep on the server side
--- PASS: TestReadRow_Generic_DeadlineExceeded (2.01s)
PASS
ok  	github.com/googleapis/cloud-bigtable-clients-test/tests	3.258s

@bhshkh bhshkh requested review from a team as code owners January 4, 2024 22:08
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: bigtable Issues related to the Bigtable API. labels Jan 4, 2024
Copy link
Contributor

@kolea2 kolea2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but can we make the PR description what the fix is for (ie why was the conformance test previously failing?)

@bhshkh
Copy link
Contributor Author

bhshkh commented Jan 24, 2024

lgtm, but can we make the PR description what the fix is for (ie why was the conformance test previously failing?)

Updated

@bhshkh bhshkh merged commit 092ee0b into googleapis:main Jan 24, 2024
9 of 12 checks passed
@bhshkh bhshkh deleted the fix/cbt-conf-test-read-row-1 branch January 24, 2024 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the Bigtable API. size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants