Bigtable 9b. Implement ReadRows row merging logic. by igorbernstein2 · Pull Request #2914 · googleapis/google-cloud-java

igorbernstein2 · 2018-02-16T01:05:09Z

This PR sets the stage for ReadRows by implementing the row merging in a RowMergingCallable. In a future commit, this callable will be part of a chain that will implement the ReadRowsCallable chain. For now its disconnected.

The implementation relies on ReframingResponseObserver to handle flow control and integrates with it by implementing the Reframer interface in RowMerger.
The RowMerger, simply glues the Reframer api to the StateMachine.
The StateMachine contains all of the logic of merging rows.

This has not been reviewed yet by anyone on the Bigtable team.
@kevinsi4508 can you take a look at this when you have a (very long) moment?

For reference the reframing logic was merged in this PR:
#2907

This PR sets the stage for ReadRows by implementing the row merging in a RowMergingCallable. In a future commit, this callable will be part of a chain that will implement the ReadRowsCallable chain. For now its disconnected. The implementation relies on ReframingResponseObserver to handle flow control and integrates with it by implementing the Reframer interface in RowMerger. The RowMerger, simply glues the Reframer api to the StateMachine. The StateMachine contains all of the logic of merging rows.

kevinsi4508

Some comments, more to come.

Sign in to view

+ if (byteA == byteB) {
+ continue;
+ }
+ return byteA < byteB ? -1 : 1;


Sign in to view

+ if (sizeA == sizeB) {
+ return 0;
+ }
+ return sizeA < sizeB ? -1 : 1;


Sign in to view

+ buffer = response;
+ nextChunk = 0;
+
+ // If the server sends a scan heartbeat, wrap it in a synthetic row that will be be filtered out


Sign in to view

+ nextChunk = 0;
+
+ // If the server sends a scan heartbeat, wrap it in a synthetic row that will be be filtered out
+ // after the resume logic.


Sign in to view

+
+ @Override
+ public boolean hasFullFrame() {
+ return nextRow != null || readNextRow();


Sign in to view

+ * <p>Package-private for internal use.
+ */
+@InternalApi
+class RowMerger<RowT> implements Reframer<RowT, ReadRowsResponse> {


Sign in to view

+
+ @Override
+ public boolean hasPartialFrame() {
+ return hasFullFrame() || stateMachine.isRowInProgress();


Sign in to view

+
+ @Override
+ public RowT pop() {
+ RowT row = nextRow;


igorbernstein2 · 2018-02-20T15:45:52Z

Ok, I think I addressed all of Kevin's feedback. @pongad @garrettjonesgoogle, this should be ready for you

kevinsi4508 · 2018-02-21T13:59:54Z

LGTM, please ask Gary to take a look as well.

igorbernstein2 · 2018-02-21T14:14:07Z

@garye can you take a look as well?

Sign in to view

+ *
+ * <ul>
+ * <li>Logical rows that were constructed using the {@link RowBuilder}
+ * <li>Special marker rows that represent resumption points that were sent by Last scanned row


Sign in to view

+ /**
+ * Feeds the last scanned serverside key into the state machine. The adapter will wrap this key in
+ * a special marker row that can be used downstream for efficient resume. It is the callers
+ * responsibility to eventually filter out this row.


Sign in to view

+
+ /**
+ * Feeds the last scanned serverside key into the state machine. The adapter will wrap this key in
+ * a special marker row that can be used downstream for efficient resume. It is the callers


Sign in to view

+ * <dt>Valid states:
+ * <dd>{@link StateMachine#AWAITING_NEW_ROW}
+ * <dt>Resulting states:
+ * <dd>{@link StateMachine#AWAITING_ROW_CONSUME}


Sign in to view

+
+ /**
+ * Returns the last completed row and transitions to awaiting a new row.
+ *


Sign in to view

+
+ @Override
+ State handleChunk(CellChunk chunk) {
+ validate(!chunk.getResetRow(), "AWAITING_NEW_ROW: can't reset");


Sign in to view

+ rowKey = chunk.getRowKey();
+ adapter.startRow(rowKey);
+
+ // auto transition


Sign in to view

+ }
+ adapter.finishCell();
+
+ if (!chunk.getCommitRow()) {


Sign in to view

+ }
+ };
+
+ private static final State AWAITING_ROW_CONSUME =


igorbernstein2 · 2018-02-21T19:44:22Z

@garye Thanks for reviewing, I think addressed all feedback. PTAL

pongad

A few nits but looks good as far as I can tell otherwise. I'm not qualified to fully review the state machine though :(

Sign in to view

+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.cloud.bigtable.data.v2.stub.read_rows;


Sign in to view

+
+ /**
+ * Feeds a new chunk into the sate machine. If the chunk is invalid, the state machine will throw
+ * an exception and should be used for further input.


igorbernstein2 · 2018-02-21T22:37:09Z

Please merge when ready.

#2914) * chore: Update generation configuration at Tue Mar 18 16:57:20 UTC 2025 * chore: Update generation configuration at Tue Mar 18 17:05:10 UTC 2025 * chore: Update generation configuration at Tue Mar 18 18:42:23 UTC 2025

… to v2.27.3 (#2914) * chore(deps): update dependency com.google.cloud:google-cloud-bigtable to v2.27.3 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

#2914) * chore: Update generation configuration at Tue Mar 18 16:57:20 UTC 2025 * chore: Update generation configuration at Tue Mar 18 17:05:10 UTC 2025 * chore: Update generation configuration at Tue Mar 18 18:42:23 UTC 2025

🤖 I have created a release *beep* *boop* --- ### Updating meta-information for bleeding-edge SNAPSHOT release. --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).

igorbernstein2 requested a review from pongad as a code owner February 16, 2018 01:05

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 16, 2018

igorbernstein2 added 5 commits February 15, 2018 20:09

fix doc
0f0efeb

cleanup the acceptance test
d460b5d

clean up chunk handling to have better flow
bbfdaa9

whitespace
1b489f1

use static
1f16c3c

googleapis deleted a comment Feb 16, 2018

kevinsi4508 reviewed Feb 16, 2018
View reviewed changes

googleapis deleted a comment Feb 17, 2018

fix doc
1f411a3

googleapis deleted a comment Feb 17, 2018

address feedback
04e5304

igorbernstein2 added 2 commits February 20, 2018 14:27

Add a couple tests for row merging not covered by the json
ae05a63

remove debug
5194407

garye suggested changes Feb 21, 2018
View reviewed changes

address feedback
e567e36

pongad reviewed Feb 21, 2018
View reviewed changes

rename read_rows -> readrows, fix docs
9ee63e6

pongad merged commit 7131ecc into googleapis:masterFeb 21, 2018

igorbernstein2 deleted the 09b-impl-read-rows-merging branch February 22, 2018 11:55

igorbernstein2 mentioned this pull request Feb 22, 2018
Bigtable: 10. Implementation: integrate ReadRows #2940
Merged

Uh oh!

Conversation

igorbernstein2 commented Feb 16, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevinsi4508 left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

igorbernstein2 commented Feb 20, 2018

Uh oh!

kevinsi4508 commented Feb 21, 2018

Uh oh!

igorbernstein2 commented Feb 21, 2018

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

igorbernstein2 commented Feb 16, 2018•
edited
Loading