ctubbsii commented on code in PR #4790:
URL: https://github.com/apache/accumulo/pull/4790#discussion_r1719092094
##########
server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java:
##########
@@ -326,7 +333,8 @@ private Map<UUID,TServerInstance>
removeEntriesInUse(Map<TServerInstance,Set<UUI
}
}
}
- }
+ });
+ store.close();
Review Comment:
Should this be in a finally block? A lot of errors could happen in the prior
block.
##########
server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java:
##########
@@ -82,38 +85,43 @@ public class GarbageCollectWriteAheadLogs {
*/
GarbageCollectWriteAheadLogs(final ServerContext context, final
VolumeManager fs,
final LiveTServerSet liveServers, boolean useTrash) {
- this.context = context;
- this.fs = fs;
- this.useTrash = useTrash;
- this.liveServers = liveServers;
- this.walMarker = new WalStateManager(context);
- this.store = () -> Iterators.concat(
- TabletStateStore.getStoreForLevel(DataLevel.ROOT, context).iterator(),
- TabletStateStore.getStoreForLevel(DataLevel.METADATA,
context).iterator(),
- TabletStateStore.getStoreForLevel(DataLevel.USER, context).iterator());
+ this(context, fs, liveServers, useTrash, new WalStateManager(context),
createStore(context));
}
- /**
- * Creates a new GC WAL object. Meant for testing -- allows mocked objects.
- *
- * @param context the collection server's context
- * @param fs volume manager to use
- * @param useTrash true to move files to trash rather than delete them
- * @param liveTServerSet a started LiveTServerSet instance
- */
- @VisibleForTesting
- GarbageCollectWriteAheadLogs(ServerContext context, VolumeManager fs,
boolean useTrash,
- LiveTServerSet liveTServerSet, WalStateManager walMarker,
- Iterable<TabletLocationState> store) {
+ GarbageCollectWriteAheadLogs(final ServerContext context, final
VolumeManager fs,
+ final LiveTServerSet liveServers, boolean useTrash, final
WalStateManager walMarker,
+ final Stream<TabletLocationState> store) {
this.context = context;
this.fs = fs;
this.useTrash = useTrash;
- this.liveServers = liveTServerSet;
+ this.liveServers = liveServers;
this.walMarker = walMarker;
this.store = store;
}
+ private static Stream<TabletLocationState> createStore(final ServerContext
context) {
+ Stream<TabletLocationState> rootStream =
+ TabletStateStore.getStoreForLevel(DataLevel.ROOT, context).stream();
+ Stream<TabletLocationState> metadataStream =
+ TabletStateStore.getStoreForLevel(DataLevel.METADATA,
context).stream();
+ Stream<TabletLocationState> userStream =
+ TabletStateStore.getStoreForLevel(DataLevel.USER, context).stream();
Review Comment:
This might be possible to be abbreviated with var. I'm not sure how it will
look after formatting, though.
```suggestion
var rootStream = TabletStateStore.getStoreForLevel(DataLevel.ROOT,
context).stream();
var metadataStream =
TabletStateStore.getStoreForLevel(DataLevel.METADATA, context).stream();
var userStream = TabletStateStore.getStoreForLevel(DataLevel.USER,
context).stream();
```
--
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]
