Skip to content

Removed RunningCompactionsSummary object from Monitor#6235

Merged
dlmarion merged 5 commits intoapache:mainfrom
dlmarion:6234-remove-wrapper-object
Mar 25, 2026
Merged

Removed RunningCompactionsSummary object from Monitor#6235
dlmarion merged 5 commits intoapache:mainfrom
dlmarion:6234-remove-wrapper-object

Conversation

@dlmarion
Copy link
Contributor

This change removes the RunningCompactionsSummary in favor of just returning a list of RunningCompactionInfo. The long running compaction map was also modified to use the RunningCompactionInfo instead of the TExternalCompaction object to reduce the computation needed at request time to return the results.

Closes #6234

This change removes the RunningCompactionsSummary in favor
of just returning a list of RunningCompactionInfo. The
long running compaction map was also modified to use
the RunningCompactionInfo instead of the TExternalCompaction
object to reduce the computation needed at request time
to return the results.

Closes apache#6234
@@ -65,6 +66,7 @@ public RunningCompactionInfo(TExternalCompaction ec) {
var updates = requireNonNull(ec.getUpdates(), "Missing Thrift external compaction updates");
Copy link
Member

Choose a reason for hiding this comment

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

I think we might need to treat tec.getUpdates() == null as an empty map here now that we are constructing the RunningCompactionInfo during the collection phase in SystemInformation (.add(new RunningCompactionInfo(tec));) instead of creating that RunningCompactionInfo later when that endpoint is used. I think the TExternalCompaction may exist without an update for a short period so we should probably handle that instead of throwing a NPE.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the TExternalCompaction may exist without an update for a short period so we should probably handle that instead of throwing a NPE.

I think that's correct. I'll update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in bc14cea

@dlmarion dlmarion merged commit 32f397a into apache:main Mar 25, 2026
9 checks passed
@dlmarion dlmarion deleted the 6234-remove-wrapper-object branch March 25, 2026 19:15
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.

4.0 Monitor - Remove RunningCompactionsSummary wrapper object

2 participants