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: Out of order read rows fix #1231

Merged

Conversation

danieljbruce
Copy link
Contributor

Client changes:

Emits an error if the chunk transformer receives keys that are out of order. The chunk transformer is a stream that data passes through before it reaches the stream that has the handlers for errors and retries.

Also pulls some comparison functions into a utility file because now those functions are used by the chunk transformer to check to see if rows are in order. They are still used by the table.ts file, but are available in both places now.

Test proxy changes:

Makes it so that when the client emits an error as a result of rows that are out of order, that the error is sent back to the test runner as an rpc message instead of an rpc error. This is done to stay consistent with the test proxy in Java and allows the TestReadRows_NoRetry_OutOfOrderError test case to pass.

This commit emits an error if the chunk transformer recieves keys that are out of order. It also pulls some comparison functions into a utility file because now those functions are used by the chunk transformer too.
This commit makes it so that when the client emits an error as a result of rows that are out of order, that the error is sent back to the test runner as an rpc message instead of an rpc error. This is done to stay consistent with the test proxy in Java and allows the TestReadRows_NoRetry_OutOfOrderError test case to pass.
@danieljbruce danieljbruce requested review from a team as code owners February 3, 2023 22:17
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: bigtable Issues related to the googleapis/nodejs-bigtable API. labels Feb 3, 2023
@danieljbruce danieljbruce changed the title Out of order read rows fix chore(testproxy): Out of order read rows fix Feb 3, 2023
@danieljbruce danieljbruce changed the title chore(testproxy): Out of order read rows fix fix: Out of order read rows fix Feb 3, 2023
@danieljbruce danieljbruce added the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 6, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 6, 2023
typeof lastRowKey !== 'undefined' &&
TableUtils.lessThanOrEqualTo(newRowKey as string, lastRowKey as string)
) {
errorMessage = 'key must be strictly increasing';
Copy link
Contributor

Choose a reason for hiding this comment

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

What about a more friendly message, e.g: A row key must be strictly increasing

The error message gets changed to match what is suggested in the pull request to be similar to other errors.
@danieljbruce danieljbruce added the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 7, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 7, 2023
@danieljbruce danieljbruce merged commit 7dbaa6e into googleapis:main Feb 7, 2023
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 googleapis/nodejs-bigtable API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants