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): Retry correct mutations #11388

Merged
merged 3 commits into from
Jan 8, 2025

Conversation

bhshkh
Copy link
Contributor

@bhshkh bhshkh commented Jan 7, 2025

b/387315971

TestMutateRows_Retry_TransientErrors fails with below errors:

=== RUN   TestMutateRows_Retry_TransientErrors
    mutaterows_test.go:287: diff found (-want +got):
          (*bigtablepb.MutateRowsRequest)(Inverse(protocmp.Transform, protocmp.Message{
          	"@type": s"google.bigtable.v2.MutateRowsRequest",
          	"entries": []protocmp.Message{
          		{
          			"@type":     s"google.bigtable.v2.MutateRowsRequest.Entry",
          			"mutations": []protocmp.Message{{"@type": s"google.bigtable.v2.Mutation", "set_cell": protocmp.Message{"@type": s"google.bigtable.v2.Mutation.SetCell", "column_qualifier": []uint8("col"), "family_name": string("f"), "timestamp_micros": int64(1000), ...}}},
          			"row_key": []uint8{
          				... // 2 identical elements
          				0x77,
          				0x2d,
        - 				0x31,
        + 				0x32,
          			},
          		},
          		{
          			"@type":     s"google.bigtable.v2.MutateRowsRequest.Entry",
          			"mutations": []protocmp.Message{{"@type": s"google.bigtable.v2.Mutation", "set_cell": protocmp.Message{"@type": s"google.bigtable.v2.Mutation.SetCell", "column_qualifier": []uint8("col"), "family_name": string("f"), "timestamp_micros": int64(1000), ...}}},
          			"row_key": []uint8{
          				... // 2 identical elements
          				0x77,
          				0x2d,
        - 				0x32,
        + 				0x33,
          			},
          		},
          	},
          	"table_name": string("projects/project/instances/instance/tables/table"),
          }))
    mutaterows_test.go:292: diff found (-want +got):
          (*bigtablepb.MutateRowsRequest)(Inverse(protocmp.Transform, protocmp.Message{
          	"@type": s"google.bigtable.v2.MutateRowsRequest",
          	"entries": []protocmp.Message{
          		{
          			"@type":     s"google.bigtable.v2.MutateRowsRequest.Entry",
          			"mutations": []protocmp.Message{{"@type": s"google.bigtable.v2.Mutation", "set_cell": protocmp.Message{"@type": s"google.bigtable.v2.Mutation.SetCell", "column_qualifier": []uint8("col"), "family_name": string("f"), "timestamp_micros": int64(1000), ...}}},
          			"row_key": []uint8{
          				... // 2 identical elements
          				0x77,
          				0x2d,
        - 				0x31,
        + 				0x33,
          			},
          		},
          	},
          	"table_name": string("projects/project/instances/instance/tables/table"),
          }))
--- FAIL: TestMutateRows_Retry_TransientErrors (0.15s)

These are requests and responses sent by the mock server setup by the test
Tests send requests for 4 row mutations (row-0, row-1, row-2, row-3). The mock server returns 'Unavailable' error for row-1 and row-2.
Expected: The library should retry mutations for row-1 and row-2
Actual: The library retries mutations for row-2 and row-3
Cause: The library assumes that responses are returned in the same order of the requests. This is not true.
The server may return the response in any order. The Index field should be used to determine the request for which the response was sent. In this test, for req [row-0, row-1, row-2, row-3], the response sent by the server is [{Index: 0, Err:nil}, {Index: 3, Err:nil}, {Index: 1, Err: Unavailable}, {Index: 2, Err: Unavailable}]. The library incorrectly assumes one on one mapping of error array to req array i.e. it assumes that unavailable response was received for row-2 and row-3 and retries them.
Fix: This PR fixes the test by using Index field to determine which rows need to be retried

After fix:

=== RUN   TestMutateRows_Retry_TransientErrors
--- PASS: TestMutateRows_Retry_TransientErrors (0.12s)
PASS
ok  	github.com/googleapis/cloud-bigtable-clients-test/tests	0.137s

@bhshkh bhshkh requested review from a team as code owners January 7, 2025 21:30
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the Bigtable API. label Jan 7, 2025
@bhshkh bhshkh requested a review from gkevinzheng January 7, 2025 21:31
@bhshkh bhshkh enabled auto-merge (squash) January 8, 2025 02:09
@bhshkh bhshkh merged commit ca2c4e3 into googleapis:main Jan 8, 2025
8 of 10 checks passed
@bhshkh bhshkh deleted the fix/cbt-apply-bulk branch January 8, 2025 02:25
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants