TODO Cleanup - #6509
Conversation
Amemeda
changed the title
| /* | ||
| * TODO ACCUMULO-2938 redact key extents in this output to avoid leaking protected | ||
| * information. | ||
| */ | ||
|
|
||
| // convert each tabletId in migrations to keyExtent | ||
| Set<KeyExtent> keyExtents = migrations.stream().map(tabletId -> { | ||
| KeyExtent extent = KeyExtent.fromTabletId(tabletId); | ||
| extent.obscured(); | ||
| return extent; | ||
| }).collect(Collectors.toSet()); | ||
|
|
||
| if (log.isDebugEnabled()) { | ||
| log.debug("Sample up to 10 outstanding migrations: {}", | ||
| migrations.stream().limit(10).map(String::valueOf).collect(Collectors.joining(", "))); | ||
| keyExtents.stream().limit(10).map(String::valueOf).collect(Collectors.joining(", "))); |
There was a problem hiding this comment.
Adding these comments from the ASF accumulo channel
| String.join(", ", migrations.stream().limit(10).map(tabletId -> { | ||
| KeyExtent extent = KeyExtent.fromTabletId(tabletId); | ||
| extent.obscured(); | ||
| return extent.toString(); | ||
| }).map(String::valueOf).collect(Collectors.toSet()))); |
There was a problem hiding this comment.
| String.join(", ", migrations.stream().limit(10).map(tabletId -> { | |
| KeyExtent extent = KeyExtent.fromTabletId(tabletId); | |
| extent.obscured(); | |
| return extent.toString(); | |
| }).map(String::valueOf).collect(Collectors.toSet()))); | |
| migrations.stream() | |
| .limit(10) | |
| .map(tabletId -> { | |
| KeyExtent extent = KeyExtent.fromTabletId(tabletId); | |
| extent.obscured(); | |
| return extent.toString(); | |
| }) | |
| .collect(Collectors.joining(", ")); |
I think you can do the join right off the stream so we dont need to collect to a new set. I didnt test this code so the working code might be different
if (retryCountDownTimer.isExpired()) {
// TODO exception used for timeout is inconsistent
throw new TimedOutException(
"Failed to find servers to process scans before timeout was exceeded.");
}Does anyone know why this exception from |
| KeyExtent extent = KeyExtent.fromTabletId(tabletId); | ||
| extent.obscured(); | ||
| return extent.toString(); |
There was a problem hiding this comment.
obscured returns the obfuscated String.
| KeyExtent extent = KeyExtent.fromTabletId(tabletId); | |
| extent.obscured(); | |
| return extent.toString(); | |
| return KeyExtent.fromTabletId(tabletId).obscured(); |
No idea |
| if (retryCountDownTimer.isExpired()) { | ||
| // TODO exception used for timeout is inconsistent | ||
| throw new TimedOutException( | ||
| "Failed to find servers to process scans before timeout was exceeded."); | ||
| } |
There was a problem hiding this comment.
Removed this TODO for now. The exception used here seems consistent with what is happening: TimedOutException because the retryCountDownTimer has expired. If anything, the message may not be accurate description of why the timeout happened.
I can add it back if the todo should remain, or if a different exception/message should be used here, I can replace.
3 participants
ThrottledBalancerProblemReporter.java. Added this logic onto existing .stream in the logger that logs 10 outstanding migrations.ClientContext.java.TabletServerBatchReaderIterator.java, the exception used here does not seem inconsistent since the retryCountDownTimer expired.Removes 3 TODOs from #2699