feat(): Export partition resitration times as runtime metrics#27437
Merged
spershin merged 1 commit intoprestodb:masterfrom Mar 25, 2026
Merged
feat(): Export partition resitration times as runtime metrics#27437spershin merged 1 commit intoprestodb:masterfrom
spershin merged 1 commit intoprestodb:masterfrom
Conversation
Summary: Export time spent on various partition registration activities as query runtime metrics. To expose potential metastore regression and slowness. Differential Revision: D98159827
|
|
Contributor
Reviewer's GuideAdds runtime metrics collection for Hive metastore partition registration and statistics update operations, wiring coordinator session RuntimeStats into SemiTransactionalHiveMetastore and timing key metastore calls using new RuntimeMetricName constants. Sequence diagram for wiring RuntimeStats into metastore partition registrationsequenceDiagram
actor User
participant Coordinator
participant HiveMetadata
participant SemiTransactionalHiveMetastore
participant MetastoreContext
participant ExtendedHiveMetastore
participant RuntimeStats
User->>Coordinator: submit_query
Coordinator->>HiveMetadata: beginCreateTable(session)
HiveMetadata->>RuntimeStats: session.getRuntimeStats()
HiveMetadata->>SemiTransactionalHiveMetastore: setQueryRuntimeStats(runtimeStats)
HiveMetadata->>SemiTransactionalHiveMetastore: beginCreateTable_operations
loop commit_phase
HiveMetadata->>SemiTransactionalHiveMetastore: commitShared()
SemiTransactionalHiveMetastore->>MetastoreContext: getRuntimeStats()
MetastoreContext-->>SemiTransactionalHiveMetastore: runtimeStats
SemiTransactionalHiveMetastore->>RuntimeStats: recordWallTime(METASTORE_ADD_PARTITIONS_TIME_NANOS, addPartitions)
RuntimeStats->>ExtendedHiveMetastore: addPartitions(metastoreContext, schema, table, partitions)
ExtendedHiveMetastore-->>RuntimeStats: addPartitions_result
RuntimeStats-->>SemiTransactionalHiveMetastore: timing_recorded
end
Class diagram for updated metastore runtime metrics integrationclassDiagram
class RuntimeStats {
+recordWallTime(metricName String, operation Callable) Object
}
class RuntimeMetricName {
<<final>>
+METASTORE_ADD_PARTITIONS_TIME_NANOS String
+METASTORE_ALTER_PARTITION_TIME_NANOS String
+METASTORE_ALTER_PARTITIONS_TIME_NANOS String
+METASTORE_UPDATE_PARTITION_STATISTICS_TIME_NANOS String
+METASTORE_UPDATE_TABLE_STATISTICS_TIME_NANOS String
}
class MetastoreContext {
+getRuntimeStats() RuntimeStats
}
class ExtendedHiveMetastore {
+addPartitions(metastoreContext MetastoreContext, schemaName String, tableName String, partitions List) Object
+alterPartition(metastoreContext MetastoreContext, databaseName String, tableName String, newPartition Object) Object
+updatePartitionStatistics(metastoreContext MetastoreContext, schemaName String, tableName String, partitionName String, updateFunction Function) void
+updateTableStatistics(metastoreContext MetastoreContext, schemaName String, tableName String, updateFunction Function) void
}
class SemiTransactionalHiveMetastore {
-state State
-throwOnCleanupFailure boolean
-queryRuntimeStats RuntimeStats
+setQueryRuntimeStats(runtimeStats RuntimeStats) void
+commitShared() ConnectorCommitHandle
+AddPartitionsOperation
+AlterPartitionOperation
+UpdateStatisticsOperation
}
class HiveMetadata {
-metastore SemiTransactionalHiveMetastore
+beginCreateTable(session ConnectorSession, tableMetadata ConnectorTableMetadata, layout Optional) HiveOutputTableHandle
+beginInsert(session ConnectorSession, tableHandle ConnectorTableHandle) HiveInsertTableHandle
-beginInsertInternal(session ConnectorSession, tableHandle ConnectorTableHandle) HiveInsertTableHandle
}
class ConnectorSession {
+getRuntimeStats() RuntimeStats
}
class AddPartitionsOperation {
+execute() void
}
class AlterPartitionOperation {
+run(metastore ExtendedHiveMetastore) void
+undo(metastore ExtendedHiveMetastore) void
}
class UpdateStatisticsOperation {
+run(metastore ExtendedHiveMetastore) void
}
HiveMetadata --> SemiTransactionalHiveMetastore : uses
HiveMetadata --> ConnectorSession : uses
ConnectorSession --> RuntimeStats : provides
SemiTransactionalHiveMetastore --> RuntimeStats : holds_queryRuntimeStats
SemiTransactionalHiveMetastore --> MetastoreContext : uses
SemiTransactionalHiveMetastore --> ExtendedHiveMetastore : delegates_to
SemiTransactionalHiveMetastore *-- AddPartitionsOperation
SemiTransactionalHiveMetastore *-- AlterPartitionOperation
SemiTransactionalHiveMetastore *-- UpdateStatisticsOperation
AddPartitionsOperation --> MetastoreContext : uses
AddPartitionsOperation --> ExtendedHiveMetastore : calls_addPartitions
AddPartitionsOperation --> RuntimeMetricName : uses_constants
AlterPartitionOperation --> MetastoreContext : uses
AlterPartitionOperation --> ExtendedHiveMetastore : calls_alterPartition
AlterPartitionOperation --> RuntimeMetricName : uses_constants
UpdateStatisticsOperation --> MetastoreContext : uses
UpdateStatisticsOperation --> ExtendedHiveMetastore : calls_updateStatistics
UpdateStatisticsOperation --> RuntimeMetricName : uses_constants
RuntimeStats --> RuntimeMetricName : uses_metricName
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The mutable, shared
queryRuntimeStatsfield inSemiTransactionalHiveMetastorerelies onsetQueryRuntimeStatsbeing called on the correct instance before commit; if this class is ever reused across queries or threads, consider enforcing per-query scoping or passingRuntimeStatsexplicitly through commit APIs instead of a volatile field to avoid mis-attribution of metrics. - The new runtime metrics are wired only via
beginCreateTableandbeginInsert; if there are other write/commit paths (e.g., deletes, stats-only operations, or other SemiTransactionalHiveMetastore usages) that should contribute to the same query-level metrics, consider settingqueryRuntimeStatsfor those entry points as well to keep attribution consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The mutable, shared `queryRuntimeStats` field in `SemiTransactionalHiveMetastore` relies on `setQueryRuntimeStats` being called on the correct instance before commit; if this class is ever reused across queries or threads, consider enforcing per-query scoping or passing `RuntimeStats` explicitly through commit APIs instead of a volatile field to avoid mis-attribution of metrics.
- The new runtime metrics are wired only via `beginCreateTable` and `beginInsert`; if there are other write/commit paths (e.g., deletes, stats-only operations, or other SemiTransactionalHiveMetastore usages) that should contribute to the same query-level metrics, consider setting `queryRuntimeStats` for those entry points as well to keep attribution consistent.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
spershin
requested review from
arhimondr,
feilong-liu,
hantangwangd,
imjalpreet,
rschlussel,
rschlussel-zz and
vhsu14
and removed request for
hantangwangd,
imjalpreet and
vhsu14
arhimondr
approved these changes
Mar 25, 2026
| columnConverterProvider, | ||
| hdfsContext.getSession().map(ConnectorSession::getWarningCollector).orElse(NOOP), | ||
| hdfsContext.getSession().map(ConnectorSession::getRuntimeStats).orElseGet(RuntimeStats::new)); | ||
| queryRuntimeStats); |
Member
There was a problem hiding this comment.
Do you need to store the runtime stats as a filed? Is it possible to get it from session directly (e.g.: hdfsContext.getSession().getRuntimeStats())
Contributor
Author
There was a problem hiding this comment.
@arhimondr
According to AI the session of the hdfsContext is a different instance and any metrics dumped there won't be in the query's metrics. :(
Contributor
Author
There was a problem hiding this comment.
My early test runs showed no new metric added to the query if we use hdfsContext.
That is a very different context. Not sure we even need that.
Something could probably be improved here, but it is out of scope for what we want to achieve.
This was referenced Mar 31, 2026
15 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Export time spent on various partition registration activities as
query runtime metrics.
To expose potential metastore regression and slowness.
Differential Revision: D98159827
Release Notes
Please follow release notes guidelines and fill in the release notes below.
Summary by Sourcery
Track and export metastore partition registration and statistics update timings as query runtime metrics.
New Features:
Enhancements: