[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 >= Ack ) {
         return True;
   } else {
        return  false;
   }
   ```
   2. But 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