You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "DomGarguilo (via GitHub)" <gi...@apache.org> on 2026/06/23 17:22:22 UTC
[PR] Replace WAL state-path-Pair with new record class [accumulo]
DomGarguilo opened a new pull request, #6442:
URL: https://github.com/apache/accumulo/pull/6442
* `WalStateManager.parse()` returned a `Pair<WalState,Path>`. This PR replaces that with the new `public record WalStatePath(WalState state, Path path)` which improves readability
* ba271c63f52c576780510849169ef4d5417dfa3c completed the bulk of this work
* 0e48e31358ebacf4621639e4d382b9cdd6829938 cleaned things up by replacing a related Pair returned by another method with this new object.
* In this commit i also made an improvement by using `removeIf()` in `GarbageCollectWriteAheadLogs.removeEntriesInUse()`
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace WAL state-path-Pair with new record class [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #6442:
URL: https://github.com/apache/accumulo/pull/6442#discussion_r3481241325
##########
server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/SystemConfigCheckRunner.java:
##########
@@ -195,10 +194,10 @@ private static boolean checkZKWALsMetadata(ServerContext context) throws Excepti
return status;
}
- private static Map<TServerInstance,Set<Pair<WalStateManager.WalState,Path>>>
- gatherWalsFromZK(ServerContext context, ZooReaderWriter zrw) throws Exception {
+ private static Map<TServerInstance,Set<WalStatePath>> gatherWalsFromZK(ServerContext context,
+ ZooReaderWriter zrw) throws Exception {
final var rootWalsDir = WalStateManager.ZWALS;
- Map<TServerInstance,Set<Pair<WalStateManager.WalState,Path>>> wals = new HashMap<>();
+ Map<TServerInstance,Set<WalStatePath>> wals = new HashMap<>();
Review Comment:
If this line was moved down one, then could initialize the map with `tserverInstances.size()`
##########
server/base/src/main/java/org/apache/accumulo/server/log/WalStateManager.java:
##########
@@ -213,15 +217,15 @@ public Pair<WalState,Path> state(TServerInstance instance, UUID uuid) throws Wal
}
}
- // utility combination of getAllMarkers and state
- public Map<Path,WalState> getAllState() throws WalMarkerException {
- Map<Path,WalState> result = new HashMap<>();
+ /**
+ * @return the state and path for all WAL markers.
+ */
+ public Set<WalStatePath> getAllState() throws WalMarkerException {
+ Set<WalStatePath> result = new HashSet<>();
Review Comment:
you could initialize this with `getAllMarkers.size()`. It may not be correct, but will likely be a closer approximation to the required size and should reduce the time spent extending the set.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace WAL state-path-Pair with new record class [accumulo]
Posted by "DomGarguilo (via GitHub)" <gi...@apache.org>.
DomGarguilo merged PR #6442:
URL: https://github.com/apache/accumulo/pull/6442
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace WAL state-path-Pair with new record class [accumulo]
Posted by "DomGarguilo (via GitHub)" <gi...@apache.org>.
DomGarguilo commented on code in PR #6442:
URL: https://github.com/apache/accumulo/pull/6442#discussion_r3481902507
##########
server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/SystemConfigCheckRunner.java:
##########
@@ -195,10 +194,10 @@ private static boolean checkZKWALsMetadata(ServerContext context) throws Excepti
return status;
}
- private static Map<TServerInstance,Set<Pair<WalStateManager.WalState,Path>>>
- gatherWalsFromZK(ServerContext context, ZooReaderWriter zrw) throws Exception {
+ private static Map<TServerInstance,Set<WalStatePath>> gatherWalsFromZK(ServerContext context,
+ ZooReaderWriter zrw) throws Exception {
final var rootWalsDir = WalStateManager.ZWALS;
- Map<TServerInstance,Set<Pair<WalStateManager.WalState,Path>>> wals = new HashMap<>();
+ Map<TServerInstance,Set<WalStatePath>> wals = new HashMap<>();
Review Comment:
Yea you're right. added both suggestion changes to 2dbc77b
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org