[GitHub] [beam] jaketf commented on pull request #11596: [BEAM-9856] [*WIP DO NOT MERGE*] Optimization/hl7v2 io list messages

[GitBox]

jaketf commented on pull request #11596:
URL: https://github.com/apache/beam/pull/11596#issuecomment-623737955


   > The NPE is caused by `OffsetRangeTracker.lastAttemptedOffset` being 
unboxed in `OffsetRangeTracker::checkDone` 
[here](https://github.com/apache/beam/blob/0291976d6029b4cf4d72caa03bd3836900fb6328/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/splittabledofn/OffsetRangeTracker.java#L98).
   > 
   > 
[all](https://github.com/apache/beam/blob/0291976d6029b4cf4d72caa03bd3836900fb6328/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/splittabledofn/OffsetRangeTracker.java#L53)
 
[other](https://github.com/apache/beam/blob/0291976d6029b4cf4d72caa03bd3836900fb6328/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/splittabledofn/OffsetRangeTracker.java#L77)
 
[uses](https://github.com/apache/beam/blob/0291976d6029b4cf4d72caa03bd3836900fb6328/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/splittabledofn/OffsetRangeTracker.java#L119)
 check that `lastAttemptedOffset` is not null.
   > 
   > I'm not sure if this was intentional in the implementation of 
OffsetRangeTracker.
   
   @chamikaramj pablo said you might know about this.
   
   should check done have some conditional on `lastAttemptedOffset != null`
   e.g.
   ```java
     @Override
     public void checkDone() throws IllegalStateException {
       if (range.getFrom() == range.getTo()) {
         return;
       }
   
       if (lastAttemptedOffset != null) {
         checkState(
             lastAttemptedOffset >= range.getTo() - 1,
             "Last attempted offset was %s in range %s, claiming work in [%s, 
%s) was not attempted",
             lastAttemptedOffset,
             range,
             lastAttemptedOffset + 1,
             range.getTo());
       }
     }
   ```
   
   I'm not really familiar with what checkDone should do in the case that 
lastAttemptedOffset was null.


----------------------------------------------------------------
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:
us...@infra.apache.org