milleruntime commented on a change in pull request #2318:
URL: https://github.com/apache/accumulo/pull/2318#discussion_r739205022
##########
File path:
core/src/main/java/org/apache/accumulo/core/util/compaction/ExternalCompactionUtil.java
##########
@@ -191,29 +227,28 @@ private static ExternalCompactionId
getRunningCompactionId(HostAndPort compactor
* server context
* @return map of compactor and external compaction jobs
*/
- public static Map<HostAndPort,TExternalCompactionJob>
- getCompactionsRunningOnCompactors(ClientContext context) {
-
- final List<Pair<HostAndPort,Future<TExternalCompactionJob>>> running = new
ArrayList<>();
+ public static List<RunningCompaction>
getCompactionsRunningOnCompactors(ClientContext context) {
+ final List<RunningCompactionFuture> rcFutures = new ArrayList<>();
Review comment:
OK thanks for checking.
Some of the public static methods in `ExternalCompactionUtil` make me wonder
if we should do some refactoring of the static code in this class. I see that
method is used in a JUnit test but removing the static state usually makes it
easier to test. Maybe we could avoid using all the PowerMock stuff that was
just introduced. We spent _a _lot__ of time removing static state in 2.0 and
once it's exposed to the user, it is a lot harder to remove. Any method that
takes a context as a parameter usually means that method should live inside of
a context object instead.
--
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]
