feat: implement MirrorinAsyncTable's scan() and scanAll()

[prawilny]

IIRC we decided to be like Bigtable and accept only ScanResultConsumer (and not support AdvancedScanConsumer).
Some OOP-jitsu needs to be done so that we express this within Java type system (Bigtable just uses raw types rather than parametrized by consumer type 🤷 ).

---

This change is 

[mwalkiewicz]

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

---

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

  @Override
  public void scan(Scan scan, C c) {

c -> consumer?

---

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

          @Override
          public boolean onNext(Result result) {
            return ((ScanResultConsumer) c).onNext(result);

What is the user experience when Advanced consumer is passed? CastException? Can we do a check using Precondition (or whatever is this library called :/ ) that this specific class is passed and add a meaningful error that will explain why is Advanced not supported?

OTOH, can't we support both results consumers? if c isinstance Advanced { override and forward advanced } else if c ininstance NonAdvanced { override and forward non-advanced } else if c isinstance ScanResultConsumer { override and forward only the base interface } else throw . This way our code will be ready for bigtable implementing advanced consumer and the code won't be much more convoluted (if you move the logic to a separate method).

---

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

  @Override
  public CompletableFuture<List<Result>> scanAll(Scan scan) {
    final CompletableFuture<List<Result>> future = new CompletableFuture<>();

Why can't we just call return primary.scanAll(scan)?

---

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 896 at r1 (raw file):

nullable

do you need nullable here? I think that any() will be sufficient, because the method is not called anyways, so there won't be any null to match.

[prawilny]

Reviewable status: 1 of 3 files reviewed, 4 unresolved discussions (waiting on @mwalkiewicz)

---

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

Previously, mwalkiewicz (Mateusz Walkiewicz) wrote…

c -> consumer?

Done.

---

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

Previously, mwalkiewicz (Mateusz Walkiewicz) wrote…

What is the user experience when Advanced consumer is passed? CastException? Can we do a check using Precondition (or whatever is this library called :/ ) that this specific class is passed and add a meaningful error that will explain why is Advanced not supported?

OTOH, can't we support both results consumers? if c isinstance Advanced { override and forward advanced } else if c ininstance NonAdvanced { override and forward non-advanced } else if c isinstance ScanResultConsumer { override and forward only the base interface } else throw . This way our code will be ready for bigtable implementing advanced consumer and the code won't be much more convoluted (if you move the logic to a separate method).

Changed to accepting both kinds of consumers (and thus the errors are going to be just as if the user used Bigtable directly).

The first version of code intercepted the consumers.
Then I remembered our f2f discussion that we can skip the reference counting here (as the user would crash the database if he tried to close it before calling ScanController#terminate() or receiving ScanConsumerBase#onComplete()) and so I removed the reference counting (which complicated the code).
But I pushed the intermediary code so you can hopefully check the difference using the previous version in reviewable.

---

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

Previously, mwalkiewicz (Mateusz Walkiewicz) wrote…

Why can't we just call return primary.scanAll(scan)?

Changed it to as suggested.
Note that now scanAll() is never verified (in the previous version it used to be) using results from secondary database.

---

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 896 at r1 (raw file):

Previously, mwalkiewicz (Mateusz Walkiewicz) wrote…

nullable

do you need nullable here? I think that any() will be sufficient, because the method is not called anyways, so there won't be any null to match.

It turns out I forgot to finish the test. Fixed it and nullable indeed wasn't needed.

[prawilny]

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