[GitHub] [bookkeeper] chairmank commented on issue #1970: Exception when sending read requests

[GitBox <gi...@apache.org>]

chairmank commented on issue #1970: Exception when sending read requests
URL: https://github.com/apache/bookkeeper/issues/1970#issuecomment-477562490
 
 
   I have observed this bug with BookKeeper 4.9.0 when using the tiered storage feature of Pulsar 2.3.0.
   
   I added logging to `org.apache.bookkeeper.client.RoundRobinDistributionSchedule` and found that the `getWriteSet` method is being called with `entryId = -2`. `getWriteSet` calls the static `create` method of `WriteSetImpl`, which calls the private `reset` method, which looks like this:
   
   ```
           private void reset(int ensembleSize, int writeQuorumSize,
                              long entryId) {
               setSize(writeQuorumSize);
               for (int w = 0; w < writeQuorumSize; w++) {
                   set(w, (int) ((entryId + w) % ensembleSize));
               }
           }
   ```
   
   The `%` operator returns a negative number when the first operand is negative. This is bad because `(entryId + w) % ensembleSize)` must be an integer index into a list of available bookies in the ensemble, between 0 (inclusive) and `ensembleSize` (exclusive).
   
   We can avoid the resulting `ArrayIndexOutOfBoundsException` by replacing `%` with `java.lang.Math.floorMod`. I think that this is a good defensive change.
   
   But changing the `WriteSet` implementation is not a real solution because it doesn't address the real bug: Why is `entryId` a negative number? I don't know whether this is a bug in Pulsar or BookKeeper.

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


With regards,
Apache Git Services