ACCUMULO-4178: Updated the sender thread to include the table ids.#87
ACCUMULO-4178: Updated the sender thread to include the table ids.#87ivakegg wants to merge 1 commit intoapache:1.6from ivakegg:ACCUMULO-4178
Conversation
| Thread.currentThread().setName(msg); | ||
| StringBuilder msg = new StringBuilder(); | ||
| msg.append("sending ").append(String.format("%,d", count)).append(" mutations to ").append(String.format("%,d", mutationBatch.size())) | ||
| .append(" tablets for ").append(String.format("%,d", tableIds.size())).append(" tables ").append(tableIds).append(" to ").append(location); |
There was a problem hiding this comment.
nitpick - suggest adding surrounding tableIds with [] or (). Or, remove tableIds.size() from the output if you are going to list the tables.
There was a problem hiding this comment.
Usually the toString method for collections will enclose the output with something like []. I suspect TreeSet does this.
There was a problem hiding this comment.
TreeSet already adds []. Perhaps I should not depend on a toString method however and do the work myself to ensure it is added correctly.
There was a problem hiding this comment.
Also I agree including the number of table ids is redundant. I will remove that part....
|
Keith and Dave summed up the comments that I had to add already :) |
|
Could put the table ids at the end of the message. For example the following vs |
I do not have a real opinion either way, but if anybody has built tools to parse this message/title then I expect putting them at the end would probably be better. I will make that change. |
|
I think I have addresses all concerns here. Any votes for merging this in? |
|
LGTM 👍 I'll work on merging this in. Thanks, @ivakegg! |
|
+1 |
4 participants
No description provided.