[GitHub] [hadoop] tasanuma edited a comment on pull request #3888: HDFS-16427. Add debug log for BlockManager#chooseExcessRedundancyStriped

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

tasanuma edited a comment on pull request #3888:
URL: https://github.com/apache/hadoop/pull/3888#issuecomment-1021959105


   @tomscut IMHO, adding unit tests and assertions would be better than logging all variables to prevent inconsistent conditions between variables. For example, I suggest adding the following Preconditions to avoid inconsistencies between `candidates` and `replicasToDelete`.
   
   ```java
   Preconditions.checkArgument(candidates.containsAll(replicasToDelete));
   ```
   
   I still agree with adding more EC debug logs for not only developers but also non-developers. I prefer more descriptive logging rather than using variable names directly. And I think `targetIndex`, `found`, and `duplicated` should be omitted as they can be speculated from `storage2index`. (As I mentioned, we may want to add assertions or unit tests if they can be inconsistent.)
   
   ```java
   LOG.debug("Choose redundant EC replicas to delete from blk_{} which is located in {}", sblk.getBlockId(), storage2index);
   LOG.debug("Storages with candidate blocks to be deleted: {}", candidates);
   LOG.debug("Storages with blocks to be deleted: {}", replicasToDelete);
   ```


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org