[GitHub] jvrao commented on issue #1380: addBookieAndCheckCovered in RRQuorumCoverageSet doesn't work correctly if AcqQuorumSize is greater than (WriteQuorumSize+1)/2

[GitBox]

jvrao commented on issue #1380: addBookieAndCheckCovered in RRQuorumCoverageSet 
doesn't work correctly if AcqQuorumSize is greater than (WriteQuorumSize+1)/2
URL: https://github.com/apache/bookkeeper/issues/1380#issuecomment-387819632
 
 
   I think @reddycharan 's point is very simple.
   
   1. The logic of checkCovered() is not correct.
   ```
             if (nodesNotCovered >= ackQuorumSize || (nodesOkay == 0 && 
nodesUninitialized > 0)) {
                       return false;
              }
   ```
         The negative logic is not correct. We should be checking
   ```
   
   if (nodesOK >= AckQ ) {
         return True;
   } else {
        return  false;
   }
   ```
   2. Being said that, if we look at the usage of this, it is used only in 
readLac. So as far as ReadLac, we are OK as long as we have one valid response 
from that write quorum. If that is agreed upon behavior, do we really need a 
class and checkCovered() response?
   
   @sijie @ivankelly ?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services