pabloem commented on a change in pull request #13313:
URL: https://github.com/apache/beam/pull/13313#discussion_r523120023



##########
File path: 
sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/splittabledofn/RestrictionTracker.java
##########
@@ -49,12 +52,20 @@
    *       call to this method).
    *   <li>{@link RestrictionTracker#checkDone} MUST succeed.
    * </ul>
+   *
+   * The API is required to be implemented.

Review comment:
       ```suggestion
      * This method is <b>required</b> to be implemented.
   ```

##########
File path: 
sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/splittabledofn/RestrictionTracker.java
##########
@@ -49,12 +52,20 @@
    *       call to this method).
    *   <li>{@link RestrictionTracker#checkDone} MUST succeed.
    * </ul>
+   *
+   * The API is required to be implemented.
    */
   public abstract boolean tryClaim(PositionT position);
 
   /**
    * Returns a restriction accurately describing the full range of work the 
current {@link
    * DoFn.ProcessElement} call will do, including already completed work.
+   *
+   * <p>The current restriction returned by method may be updated dynamically 
due to due to
+   * concurrent invocation of other methods of the {@link RestrictionTracker}, 
For example, {@link
+   * RestrictionTracker#trySplit(double)}.
+   *
+   * <p>This API is required to be implemented.

Review comment:
       ```suggestion
      * <p> This method is <b>required</b> to be implemented.
   ```

##########
File path: 
sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/splittabledofn/RestrictionTracker.java
##########
@@ -116,6 +135,8 @@
    * <p>It is valid to return {@link IsBounded#BOUNDED} after returning {@link 
IsBounded#UNBOUNDED}
    * once the end of a restriction is discovered. It is not valid to return 
{@link
    * IsBounded#UNBOUNDED} after returning {@link IsBounded#BOUNDED}.
+   *
+   * <p>The API is required to be implemented.

Review comment:
       ```suggestion
      * <p> This method is <b>required</b> to be implemented.
   ```

##########
File path: 
sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/splittabledofn/RestrictionTracker.java
##########
@@ -94,10 +108,15 @@
   public abstract @Nullable SplitResult<RestrictionT> trySplit(double 
fractionOfRemainder);
 
   /**
-   * Called by the runner after {@link DoFn.ProcessElement} returns.
+   * Checks whether the restriction has been fully processed.
+   *
+   * <p>Called by the SDK harness after {@link DoFn.ProcessElement} returns.
    *
    * <p>Must throw an exception with an informative error message, if there is 
still any unclaimed
    * work remaining in the restriction.
+   *
+   * <p>This API is required to be implemented in order to make sure no data 
loss during SDK

Review comment:
       ```suggestion
      * <p>This method is <b>required</b> to be implemented in order to prevent 
data loss during SDK
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to