apurtell commented on code in PR #4338:
URL: https://github.com/apache/hbase/pull/4338#discussion_r848600124
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java:
##########
@@ -381,34 +377,46 @@ protected final List<Path> compact(final
CompactionRequestImpl request,
} else {
Closeables.close(scanner, true);
}
- if (!finished && writer != null) {
- abortWriter();
+ if (progress != null) {
+ synchronized (this.progress) {
+ this.progress.remove(progress);
+ }
+ }
+ if (writer != null) {
Review Comment:
Yes, but see the asserts below:
assert finished : "We should have exited the method on all error paths";
assert writer != null : "Writer should be non-null if no error";
These were in place even without a `return` here in the previous code. I can
add it. It does seem it was (and is) incorrect.
When we drop through to the end of this method there is a finished `writer`
we can commit, or so this claims. I will convert those assertions to throws of
`IllegalStateException` so we can see if these invariants are not holding even
when assertions are not enabled.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
