feat: 2.x - rewrite Increment and Append as Put in batch #116

mwalkiewicz · 2021-10-25T11:05:41Z

This change is

prawilny

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mwalkiewicz)

bigtable-hbase-mirroring-client-1.x-parent/bigtable-hbase-mirroring-client-1.x/src/main/java/com/google/cloud/bigtable/mirroring/hbase1_x/utils/BatchHelpers.java, line 275 at r1 (raw file):

  @SuppressWarnings("unchecked")
  public static <ActionType extends Row> List<ActionType> rewriteIncrementsAndAppendsAsPuts(

Note: it may conflict with generalBatch PR

bigtable-hbase-mirroring-client-1.x-parent/bigtable-hbase-mirroring-client-1.x/src/main/java/com/google/cloud/bigtable/mirroring/hbase1_x/utils/BatchHelpers.java, line 293 at r1 (raw file):

  }

  public static Put makePutFromResult(Result result) {

it's already extracted into OperationUtils on master (unless it's been moved elsewhere)

bigtable-hbase-mirroring-client-2.x-parent/bigtable-hbase-mirroring-client-2.x/src/test/java/com/google/cloud/bigtable/mirroring/hbase2_x/TestMirroringAsyncTable.java, line 779 at r1 (raw file):

  }

  private void assertPutsAreEqual(Put expectedPut, Put value) {

It's already extracted in TestHelpers.java

mwalkiewicz

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @prawilny)

bigtable-hbase-mirroring-client-1.x-parent/bigtable-hbase-mirroring-client-1.x/src/main/java/com/google/cloud/bigtable/mirroring/hbase1_x/utils/BatchHelpers.java, line 293 at r1 (raw file):

Previously, prawilny (Adam Czajkowski) wrote…

it's already extracted into OperationUtils on master (unless it's been moved elsewhere)

Done, good catch.

bigtable-hbase-mirroring-client-2.x-parent/bigtable-hbase-mirroring-client-2.x/src/test/java/com/google/cloud/bigtable/mirroring/hbase2_x/TestMirroringAsyncTable.java, line 779 at r1 (raw file):

Previously, prawilny (Adam Czajkowski) wrote…

It's already extracted in TestHelpers.java

Do you remember what did you refer to in this comment?

prawilny

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @mwalkiewicz and @prawilny)

bigtable-hbase-mirroring-client-2.x-parent/bigtable-hbase-mirroring-client-2.x/src/test/java/com/google/cloud/bigtable/mirroring/hbase2_x/TestMirroringAsyncTable.java, line 779 at r1 (raw file):

Previously, mwalkiewicz (Mateusz Walkiewicz) wrote…

Do you remember what did you refer to in this comment?

TestHelpers.assertPutsAreEqual, I think.
Link to current master:

java-bigtable-hbase-1/bigtable-hbase-mirroring-client-1.x-parent/bigtable-hbase-mirroring-client-1.x/src/test/java/com/google/cloud/bigtable/mirroring/hbase1_x/TestHelpers.java

Line 171 in ac5c153

public static void assertPutsAreEqual(

mwalkiewicz

Reviewable status: 0 of 5 files reviewed, all discussions resolved (waiting on @prawilny)

bigtable-hbase-mirroring-client-2.x-parent/bigtable-hbase-mirroring-client-2.x/src/test/java/com/google/cloud/bigtable/mirroring/hbase2_x/TestMirroringAsyncTable.java, line 779 at r1 (raw file):

Previously, prawilny (Adam Czajkowski) wrote…

TestHelpers.assertPutsAreEqual, I think.
Link to current master:

java-bigtable-hbase-1/bigtable-hbase-mirroring-client-1.x-parent/bigtable-hbase-mirroring-client-1.x/src/test/java/com/google/cloud/bigtable/mirroring/hbase1_x/TestHelpers.java

Line 171 in ac5c153

public static void assertPutsAreEqual(

OK, it was already removed.

prawilny

Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @mwalkiewicz)

The base branch was changed.

prawilny

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @mwalkiewicz)

… review comments (googleapis#3347) * chore: revert review comments * feat: add MirroringOperationException exception markers (#125) * feat: concurrent writes in MirroringBufferedMutator (#80) * refactor: implement multiple argument operations on MirroringAsyncTable with specific operations rather than batch() (#75) * feat: implement MirroringAsyncTable#getName() (#132) * feat: use Logger rather than stdout in DefaultMismatchDetector (#128) * feat: synchronous writes (#88) * fix: implement heapSize method for RowCell (#111) * feat: FlowController accounts for memory usage (#137) * refactor: remove Configuration as a base of MirroringConfiguration (#127) * feat: MirroringAsyncBufferedMutator (#81) * refactor: rename WRITE_MISMATCH to SECONDARY_WRITE_ERROR (#138) * fix: BufferedMutator close() waits for all secondary flushes to finish (#110) * feat: 2.x reads sampling (#114) * refactor: make MirroringResultScanner synchronize on itself rather than MirroringTable (#134) * ConcurrentBufferedMutator integration tests (#135) * feat: add synchronous MirroringConnection to 2.x (#109) * fix: MirroringConnection in 2.x failed to compile (#139) * fix: fix BufferedMutator ITs (#140) * feat: run 1.x integration tests on MirroringConnection etc. from 2.x (#108) * feat: 2.x - rewrite Increment and Append as Put in batch (#116) * fix: fix build (#142) * refactor: minor fixes after review (#117) * feat: MirroringAsyncTable#getScanner() (#58) * test: 2.x integration tests (#112) * feat: implement MirroringAsyncBufferedMutatorBuilder (#144) * feat: log rows and values in DefaultMismatchDetector (#129) * fix: ITs - add expected parameter to MismatchDetectors (#153) * fix: force Append and Increment to return results and discard that result before returning it to user (#136) * fix: review fixes in utils * fix: review fixes in BufferedMutator * fix: review fixes in Faillog * fix: fixed reference counting * fix: review fixes in FlowController * fix: review fixes in metrics * fix: review fixes in verification * fix: Review fixes in MirroringTable * fix: review fixes in HBase 2.x client * fix: fixes in ITs * feat: MirrorinAsyncTable: scan(), scanAll() (#131) * fix: review fixes in tests * feat: MirroringConnection: timeout in close() and abort() (#133) * feat: better mismatch detection of scan results (#130) * feat: quickstart (#105) * fix: 2.x scan ITs (#158) * fix: DefaultMismatchDetector tests (#157) * fix: ConcurrentBufferedMutator waits for both flushes to finish before closing (#161) * fix: additional minor fixes after review (#163) * fix: BufferedMutator review fixes (#164) - Simplify #flush(). - Add javadocs. - (sequential) Fix flush() exception handling. - (sequential) Move error handling to a separate inner class. * fix: PR fixes * fix: report zeroed error metrics after successful operations * fix: prepend MismatchDetectorCounter with Test to better reflect its purpose * feat: Client-side timestamping (#165) * fix: reduce timeout in TestBlocking to make the tests run faster * fix: asyncClose -> closePrimaryAndScheduleSecondaryClose * fix: remove unused Batcher#throwBatchDataExceptionIfPresent * fix: remove unused Comparators#compareRows * fix: extract failedReads from MatchingSuccessfulReadsResults to reduce confusion * feat: remove unused MirroringTracer from FailedMutationLogger * fix: MirroringAsyncBufferedMutator - test if failed mutation is passed to secondary write error consumer * fix: TestMirroringAsyncTableInputModification typo fix * fix: describe user flush() in Buffered Mutator in quickstart * fix: MirroringBufferedMutator - move flush threshold from BufferedMutations to FlushSerializer * refactor: MirroringBufferedMutator#close() - use AccumulatedExceptions insteand of List<Exception> * BufferedMutator - add close timeout * AsyncBufferedMutator - add close timeout * fix: remove stale addSecondaryMutation comment * fix: add a comment that addSecondaryMutation handles failed writes * fix: unify implementations of flushBufferedMutatorBeforeClosing * fix: BufferedMutator - throw exceptions on close * fix: BufferedMutator - add comment explaining that chain of flush operations is created * fix: BufferedMutator - clarify comments * fix: Concurrent BufferedMutator - fix throwFlushExceptionIfAvailable * fix: explain why flush is called in Sequential BufferedMutator test * fix: TestConcurrentMirroringBufferedMutator - make waiting for calls explicit * refactor: BufferedMutator rename scheduleFlushAll() to scheduleFlush() * refactor: make FlushSerializer non-static * fix: BufferedMutator - use HierarchicalReferenceCounter * feat: Add MirroringConnection constructor taking MirroringConfiguration * refactor: move releaseReservations to finally * fix: use less convoluted example in lastFlushFutures description * fix: merge small Timeestamper files into a single file * fix: add a comment explaining which exceptions are forwarded to the user and why in SequentialMirroringBufferedMutator * fix: use UnsupportedOperationException instead of RuntimeException when forbidden mutation type is encountered * fix: add comment explaining why batch is complicated * fix: add a TODO to implement point writes without batch Co-authored-by: Mateusz Walkiewicz <mwalkiewicz@unoperate.com> Co-authored-by: Adam Czajkowski <prawilny@unoperate.com> Co-authored-by: Kajetan Boroszko <kajetan@unoperate.com>

mwalkiewicz force-pushed the mw/2.x-read-sampling/1 branch from 9b7152c to 37649bc Compare October 25, 2021 12:29

mwalkiewicz force-pushed the mw/2.x-batch-put-append/1 branch from 79cfb55 to 756bcfb Compare October 25, 2021 12:29

mwalkiewicz linked an issue Oct 25, 2021 that may be closed by this pull request

2.x: rewrite Increment and Append as Put in batch() #82

Closed

prawilny reviewed Oct 26, 2021

View reviewed changes

prawilny force-pushed the mw/2.x-read-sampling/1 branch from 37649bc to 85664b3 Compare October 27, 2021 12:11

prawilny mentioned this pull request Nov 3, 2021

fix: force Append and Increment to return results and discard that result before returning it to user #136

Merged

mwalkiewicz force-pushed the mw/2.x-read-sampling/1 branch from 85664b3 to 717d0dd Compare November 5, 2021 08:52

mwalkiewicz force-pushed the mw/2.x-batch-put-append/1 branch from 756bcfb to f455395 Compare November 5, 2021 09:18

mwalkiewicz force-pushed the mw/2.x-read-sampling/1 branch 2 times, most recently from 8982701 to d1ce8ca Compare November 5, 2021 16:06

mwalkiewicz closed this Nov 5, 2021

mwalkiewicz force-pushed the mw/2.x-batch-put-append/1 branch from f455395 to d1ce8ca Compare November 5, 2021 16:15

mwalkiewicz reopened this Nov 5, 2021

mwalkiewicz force-pushed the mw/2.x-batch-put-append/1 branch from 6da4d1d to 3d3c82c Compare November 5, 2021 16:23

mwalkiewicz commented Nov 5, 2021

View reviewed changes

mwalkiewicz force-pushed the mw/2.x-batch-put-append/1 branch from 3d3c82c to ae570fe Compare November 5, 2021 16:29

prawilny reviewed Nov 9, 2021

View reviewed changes

mwalkiewicz commented Nov 9, 2021

View reviewed changes

prawilny self-requested a review November 9, 2021 12:50

prawilny previously approved these changes Nov 9, 2021

View reviewed changes

feat: 2.x - rewrite Increment and Append as Put in batch

bdf9bcc

mwalkiewicz force-pushed the mw/2.x-batch-put-append/1 branch from ae570fe to bdf9bcc Compare November 9, 2021 13:16

mwalkiewicz changed the base branch from mw/2.x-read-sampling/1 to master November 9, 2021 13:16

prawilny approved these changes Nov 9, 2021

View reviewed changes

mwalkiewicz merged commit f4ef97b into master Nov 10, 2021

dopiera pushed a commit that referenced this pull request Dec 2, 2021

feat: 2.x - rewrite Increment and Append as Put in batch (#116)

7484224

dopiera pushed a commit that referenced this pull request Dec 2, 2021

feat: 2.x - rewrite Increment and Append as Put in batch (#116)

0741085

dopiera pushed a commit that referenced this pull request Dec 2, 2021

feat: 2.x - rewrite Increment and Append as Put in batch (#116)

bd068d9

dopiera pushed a commit that referenced this pull request May 11, 2022

feat: 2.x - rewrite Increment and Append as Put in batch (#116)

1679f46

dopiera pushed a commit that referenced this pull request May 11, 2022

feat: 2.x - rewrite Increment and Append as Put in batch (#116)

2d40a92

dopiera pushed a commit that referenced this pull request May 11, 2022

feat: 2.x - rewrite Increment and Append as Put in batch (#116)

a2bdbb7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: 2.x - rewrite Increment and Append as Put in batch #116

feat: 2.x - rewrite Increment and Append as Put in batch #116

Uh oh!

mwalkiewicz commented Oct 25, 2021 •

edited by dopiera

Loading

Uh oh!

prawilny left a comment

Uh oh!

mwalkiewicz left a comment

Uh oh!

prawilny left a comment

Uh oh!

mwalkiewicz left a comment

Uh oh!

prawilny left a comment

Uh oh!

prawilny left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat: 2.x - rewrite Increment and Append as Put in batch #116

feat: 2.x - rewrite Increment and Append as Put in batch #116

Uh oh!

Conversation

mwalkiewicz commented Oct 25, 2021 • edited by dopiera Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prawilny left a comment

Choose a reason for hiding this comment

Uh oh!

mwalkiewicz left a comment

Choose a reason for hiding this comment

Uh oh!

prawilny left a comment

Choose a reason for hiding this comment

Uh oh!

mwalkiewicz left a comment

Choose a reason for hiding this comment

Uh oh!

prawilny left a comment

Choose a reason for hiding this comment

Uh oh!

prawilny left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mwalkiewicz commented Oct 25, 2021 •

edited by dopiera

Loading