kevinrr888 opened a new pull request, #5594: URL: https://github.com/apache/accumulo/pull/5594
closes #5584 **Which impls of `FileSKVIterator` are expected to support `estimateOverlappingEntries()`?** `estimateOverlappingEntries()` is only called once, and that is as part of each compaction job: https://github.com/apache/accumulo/blob/4a62e8161b412357422f17b3f373343e4bf1a3c9/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java#L624-L630 The `reader` created here is created by `build()` which returns `openReader()` which has 3 impls: * `DispatchingFileFactory.openReader()` - Returns a `FileSKVIterator` which is a result of `MapFileOperations.openReader()` or `RFileOperations.openReader()`. - If bloom is configured for the table, will wrap this `FileSKVIterator` in a `BloomFilterLayer.Reader`, so `estimateOverlappingEntries()` for `BloomFilterLayer.Reader` ultimately needs to call `estimateOverlappingEntries()` on the reader it's wrapping. This is the functionality now from #5518 * `MapFileOperations.openReader()` - Always results in `UnsupportedOperationException` * `RFileOperations.openReader()` - Always returns `RFile.Reader` as the `FileSKVIterator`, which supports `estimateOverlappingEntries()` So, we need `estimateOverlappingEntries()` implemented only for `BloomFilterLayer.Reader` and `RFile.Reader`. This is the current functionality, so no further changes are needed, and #5584 can be closed as complete. While looking at this, I noticed `MapFileIterator` exists now only as an impl which throws `UnsupportedOperationException`. This seemed unneccessary and made the uses of `MapFileIterator` appear more than what they were. Further, this class is not in the public API. So, I deleted the class. Does others agree that #5584 can be closed? Thoughts on deleting `MapFileIterator`? -- 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: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
