address feedback

Author: igorbernstein2

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java

@@ -81,7 +81,7 @@ public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings)
             .setCredentialsProvider(settings.getCredentialsProvider());
 
     // ReadRow retries are handled in the overlay: disable retries in the base layer (but make
-    // sure to preserve the exception callable settings.
+    // sure to preserve the exception callable settings).
     baseSettingsBuilder
         .readRowsSettings()
         .setSimpleTimeoutNoRetries(Duration.ofHours(2))

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsResumptionStrategy.java

@@ -60,7 +60,8 @@ public StreamResumptionStrategy<ReadRowsRequest, RowT> createNew() {
   public void onProgress(RowT response) {
     // Last key can come from both the last processed row key and a synthetic row marker. The
     // synthetic row marker is emitted when the server has read a lot of data that was filtered out.
-    // So it can trim the start of the scan, but does not contribute to the row limit.
+    // The row marker can be used to trim the start of the scan, but does not contribute to the row
+    // limit.
     lastKey = rowAdapter.getKey(response);
     if (!rowAdapter.isScanMarkerRow(response)) {
       // Only real rows count towards the rows limit.
@@ -79,15 +80,15 @@ public void onProgress(RowT response) {
   @Override
   public ReadRowsRequest getResumeRequest(ReadRowsRequest request) {
     // An empty lastKey means that we have not successfully read the first row,
-    // resume with the original request object.
+    // so resume with the original request object.
     if (lastKey.isEmpty()) {
       return request;
     }
 
     ReadRowsRequest originalRequest = request;
 
-    // Special case: empty query implies full table scan, make this explicit by adding an unbounded
-    // range to the request
+    // Special case: empty query implies full table scan, so make this explicit by adding an
+    // unbounded range to the request
     if (request.getRows().getRowKeysList().isEmpty()
         && request.getRows().getRowRangesList().isEmpty()) {
 
@@ -115,17 +116,17 @@ public ReadRowsRequest getResumeRequest(ReadRowsRequest request) {
 
       switch (rowRange.getEndKeyCase()) {
         case END_KEY_CLOSED:
-          if (ByteStringComparator.INSTANCE.compare(rowRange.getEndKeyClosed(), lastKey) <= 0) {
-            continue;
-          } else {
+          if (ByteStringComparator.INSTANCE.compare(rowRange.getEndKeyClosed(), lastKey) > 0) {
             rowRangeBuilder.setEndKeyClosed(rowRange.getEndKeyClosed());
+          } else {
+            continue;
           }
           break;
         case END_KEY_OPEN:
-          if (ByteStringComparator.INSTANCE.compare(rowRange.getEndKeyOpen(), lastKey) <= 0) {
-            continue;
-          } else {
+          if (ByteStringComparator.INSTANCE.compare(rowRange.getEndKeyOpen(), lastKey) > 0) {
             rowRangeBuilder.setEndKeyOpen(rowRange.getEndKeyOpen());
+          } else {
+            continue;
           }
           break;
         case ENDKEY_NOT_SET:
@@ -160,8 +161,8 @@ public ReadRowsRequest getResumeRequest(ReadRowsRequest request) {
     }
 
     // Edge case: retrying a fulfilled request.
-    // A fulfilled request is one that has had all of its row keys and ranges fulfilled or if it had
-    // a row limit, has seen enough rows. These requests are replaced with a marker request that
+    // A fulfilled request is one that has had all of its row keys and ranges fulfilled, or if it
+    // had a row limit, has seen enough rows. These requests are replaced with a marker request that
     // will be handled by ReadRowsRetryCompletedCallable. See docs in ReadRowsRetryCompletedCallable
     // for more details.
     if ((rowSetBuilder.getRowRangesCount() == 0 && rowSetBuilder.getRowKeysCount() == 0)

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsRetryCompletedCallable.java

@@ -28,8 +28,9 @@
  * an OK response.
  *
  * <p>This callable works in tandem with {@link ReadRowsResumptionStrategy}, which will send a
- * {@link #FULFILLED_REQUEST_MARKER} to be processed by this callable. Which will promptly notify
- * the {@link ResponseObserver} that the stream has been successfully compeleted.
+ * {@link #FULFILLED_REQUEST_MARKER} to be processed by this callable. Upon receiving the {@link
+ * #FULFILLED_REQUEST_MARKER}, this callable will promptly notify the {@link ResponseObserver} that
+ * the stream has been successfully compeleted.
  *
  * <p>This class is considered an internal implementation detail and not meant to be used by
  * applications.

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/package-info.java

@@ -17,9 +17,9 @@
  * Implementation details for {@link
  * com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub#readRowsCallable()}.
  *
- * <p>The ReadRows protocol is optimized for transmission and cannot be consumed directly. This
- * package implements significant customizations on top of the raw GAPIC generated stub to handle
- * row merging and retries.
+ * <p>The ReadRows protocol is optimized for transmission and is not designed to be consumed
+ * directly. This package implements significant customizations on top of the raw GAPIC generated
+ * stub to handle row merging and retries.
  *
  * <ul>
  *   <li>ReadRowsUserCallable: Creates protobuf {@link com.google.bigtable.v2.ReadRowsRequest}s from
@@ -30,7 +30,8 @@
  *       to be filtered out.
  *   <li>RowMerger (+ helpers): Implements resuming retries for gax's Callables#retrying middleware.
  *   <li>FilterMarkerRowsCallable: Filters out marker rows that are used for efficient retry
- *       resumes. This is necessary because
+ *       resumes. The marker is an internal implementation detail to communicate state to the
+ *       RowMerger and should not be exposed to the caller.
  * </ul>
  *
  * <p>This package is considered an internal implementation detail and is not meant to be used by