DomGarguilo commented on code in PR #4452:
URL: https://github.com/apache/accumulo/pull/4452#discussion_r1580058475
##########
core/src/main/java/org/apache/accumulo/core/file/FileSKVIterator.java:
##########
@@ -33,6 +34,16 @@ public interface FileSKVIterator extends
InterruptibleIterator, AutoCloseable {
DataInputStream getMetaStore(String name) throws IOException,
NoSuchMetaStoreException;
+ /**
+ * Returns an estimate of the number of entries that overlap the given
extent. This is an estimate
+ * because the extent may or may not entirely overlap with each of the index
entries included in
+ * the count. Will never underestimate but may overestimate.
+ *
+ * @param extent the key extent
+ * @return the estimate
+ */
+ long estimateEntries(KeyExtent extent) throws IOException;
Review Comment:
Would including the word `overlap` or `overlapping` in the method name make
it more self-evident? Maybe something like `estimateOverlappingEntries`?
##########
test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompactionProgressIT.java:
##########
@@ -148,6 +230,10 @@ private void checkRunning() throws TException {
progressList.add(EC_PROGRESS.HALF);
} else if (rci.progress > 75 && rci.progress <= 100) {
progressList.add(EC_PROGRESS.THREE_QUARTERS);
+ } else {
+ progressList.add(EC_PROGRESS.INVALID);
+ log.warn("An invalid progress {} has been seen. This should
never occur.",
+ rci.progress);
Review Comment:
I think it would be nice to either fail the test here instead of adding it
to the list, or where all the progress states are checked at the end, check
them all by using Junits `assertAll()` that way it will alert us if multiple
asserts fail instead of just stop at the first failure.
--
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]
