[GitHub] [cassandra] dcapwell commented on pull request #2066: Cassandra 18110

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

dcapwell commented on PR #2066:
URL: https://github.com/apache/cassandra/pull/2066#issuecomment-1377795911

   > and also noticed that
   org.apache.cassandra.streaming.StreamingState.Sessions#receivedBytesPercent and org.apache.cassandra.streaming.StreamingState.Sessions#sentBytesPercent are not used anywhere.
   
   Removed the dead code
   
   > Also interesting that div(0,0) returns BigDecimal.ZERO, not BigDecimal.ONE... as in this usage you're 100% done sending 0 files.
   
   `div` is only used by `progress`, which is 
   
   ```
   return div(bytesSent + bytesReceived, bytesToSend + bytesToReceive);
   ```
   
   There are 2 cases where `bytesToSend + bytesToReceive == 0`
   
   1) not seen any PREPARE acks; we don't know what is going to be streamed yet
   2) nothing to stream; not sure how common this is... Maybe preview repair hits this, else it would be instance doesn't have anything for the range?
   
   I left a comment to Abe (in here or slack don't remember); the fields are set to `0` on init, so can't tell if #1 has happened yet, so can't tell if we are done (nothing to stream) or not started yet...
   
   Now, how critical is this?  The only usage of `progress` is 
   
   ```
   // StreamingState
   public float progress()
       {
           switch (status)
           {
               case INIT:
                   return 0;
               case START:
                   return Math.min(0.99f, sessions().progress().floatValue());
               case SUCCESS:
               case FAILURE:
                   return 1;
               default:
                   throw new AssertionError("unknown state: " + status);
           }
       }
   ```
   
   So, we only call `sessions().progress()` when we know the stream is started and not completed.  This means that the empty case should be SUCCESS and return `1`, so the only case is #1...
   
   Given all this, the correct value to return is `0` as we are not started yet... should `div` handle this?  I can always move higher up if desired... but `0` is still the correct answer
   


-- 
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.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org