Github user dschneider-pivotal commented on a diff in the pull request:

    https://github.com/apache/geode/pull/525#discussion_r118057642
  
    --- Diff: 
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
 ---
    @@ -485,9 +485,14 @@ public Result compactDiskStore(
                   memberCompactInfo.clear();
                 }
                 String notExecutedMembers = 
CompactRequest.getNotExecutedMembers();
    +            if (notExecutedMembers != null && 
!notExecutedMembers.isEmpty()) {
    +              notExecutedMembers = "but was not send to " + 
notExecutedMembers;
    +            } else {
    +              notExecutedMembers = "all the members";
    --- End diff --
    
    I think this log message only needs to say something if 
"notExecutedMembers" is not null and not empty.
    No need for a message in the else. You will notice that immediately after 
this code is a report of what was compacted so it seem to me too verbose to say 
"I sent the request to everyone".
    
    But it could be helpful to log a note of the members that did not receive 
the request.
    So I think you should put the log lines 493-496 inside this if and get rid 
of the else.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to