Skip to content

Replace WAL state-path-Pair with new record class#6442

Merged
DomGarguilo merged 4 commits into
apache:mainfrom
DomGarguilo:walStatePair
Jun 26, 2026
Merged

Replace WAL state-path-Pair with new record class#6442
DomGarguilo merged 4 commits into
apache:mainfrom
DomGarguilo:walStatePair

Conversation

@DomGarguilo

Copy link
Copy Markdown
Member
  • WalStateManager.parse() returned a Pair<WalState,Path>. This PR replaces that with the new public record WalStatePath(WalState state, Path path) which improves readability
    • ba271c6 completed the bulk of this work
  • 0e48e31 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()

@DomGarguilo DomGarguilo added this to the 4.0.0 milestone Jun 23, 2026
@DomGarguilo DomGarguilo self-assigned this Jun 23, 2026
* @return the state and path for all WAL markers.
*/
public Set<WalStatePath> getAllState() throws WalMarkerException {
Set<WalStatePath> result = new HashSet<>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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<>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this line was moved down one, then could initialize the map with tserverInstances.size()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea you're right. added both suggestion changes to 2dbc77b

@DomGarguilo DomGarguilo merged commit 163136d into apache:main Jun 26, 2026
8 checks passed
@DomGarguilo DomGarguilo deleted the walStatePair branch June 26, 2026 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants