Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HUDI-8882] Support Compaction/Rollback/Clean Timeline Instant Metrics in HoodieMetrics #12681

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fhan688
Copy link
Contributor

@fhan688 fhan688 commented Jan 21, 2025

Change Logs

Similar like #12466 , this PR add time instant metrics for compaction、clean、rollback:
earliest_inflight_XXX_instant
latest_completed_XXX_instant
clustering_pendingXXXCount

Also, this PR refine the whole code impl.

Impact

hudi-client-common

Risk level (write none, low medium or high below)

Low

Documentation Update

None

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

@github-actions github-actions bot added the size:M PR with lines of changes in (100, 300] label Jan 21, 2025
@hudi-bot
Copy link

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@@ -565,7 +565,7 @@ protected void postCommit(HoodieTable table, HoodieCommitMetadata metadata, Stri
// Delete the marker directory for the instant.
WriteMarkersFactory.get(config.getMarkersType(), table, instantTime)
.quietDeleteMarkerDir(context, config.getMarkersDeleteParallelism());
metrics.updateClusteringTimeLineInstantMetrics(table.getActiveTimeline());
metrics.updateTimelineInstantMetrics(table.getActiveTimeline());
Copy link
Contributor

Choose a reason for hiding this comment

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

could we rename this function to updateTableServiceInstantMetrics

}
}

private void updatePendingInstantCount(final HoodieActiveTimeline activeTimeline,
Copy link
Contributor

Choose a reason for hiding this comment

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

could we add some java docs? what does PendingInstantCount mean? and what is this metric used for?

Comment on lines +431 to +438
if (HoodieTableType.MERGE_ON_READ.name().equalsIgnoreCase(tableType)
&& LATEST_COMPLETED_COMPACTION_INSTANT_STR.equalsIgnoreCase(metricName)) {
action = HoodieActiveTimeline.COMMIT_ACTION;
}
if (HoodieTableType.COPY_ON_WRITE.name().equalsIgnoreCase(tableType)
&& LATEST_COMPLETED_CLUSTERING_INSTANT_STR.equalsIgnoreCase(metricName)) {
action = HoodieActiveTimeline.REPLACE_COMMIT_ACTION;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use switch style?
Also no need to take care of tableType, bcz MOR table also can do clustering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:M PR with lines of changes in (100, 300]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants