HIVE-29753: Add configurable WARN logging for slow getFileInfo calls in StorageBasedAuthorizationProvider#6624
HIVE-29753: Add configurable WARN logging for slow getFileInfo calls in StorageBasedAuthorizationProvider#6624Indhumathi27 wants to merge 1 commit into
Conversation
…in StorageBasedAuthorizationProvider
|
ayushtkn
left a comment
There was a problem hiding this comment.
I think this is something DFSClient should be worried about not Hive as a client, Why not use the Hadoop configs?
<property>
<name>ipc.server.log.slow.rpc</name>
<value>false</value>
<description>This setting is useful to troubleshoot performance issues for
various services. If this value is set to true then we log requests that
fall into 99th percentile as well as increment RpcSlowCalls counter.
</description>
</property>
<property>
<name>ipc.server.log.slow.rpc.threshold.ms</name>
<value>0</value>
<description>The threshold in milliseconds for logging slow rpc when ipc.server.log.slow.rpc is enabled.
Besides of being much slower than other RPC requests, an RPC request has to take at least the threshold value
defined by this property before it can be considered as slow. By default, this threshold is set to 0 (disabled).
</description>
</property>
|
@ayushtkn Thanks for the review. I agree ipc.server.log.slow.rpc is the right way at the IPC layer, but I would like to highlight the advantages of having this in Hive as well. Client-side visibility without NameNode access Operation context
Without this, even with ipc.server.log.slow.rpc enabled on the NameNode, you still cannot correlate the slow RPC to a specific HMS operation (auth check vs table scan vs partition listing). Path information for diagnosis Lower blast radius restart Threshold tuning per deployment Filesystem agnostic — works for any future FileSystem Both mechanisms complement each other — ipc.server.log.slow.rpc gives NameNode-level visibility, while this fix gives HMS-level visibility with operation context. |
If my memory treats me well, that is reconfigurable and doesn't require a Restart, Hadoop has set of reconfigurable properties, which can be reconfigured without restarting the server I do understand your use case, But I feel this is something to be dealt at the Hadoop RPC Client layer rather than the Hive as a FS Client doing this magic. If we want to implement it in Hive, which I seriously think we shouldn't, we should not just for -> start counter -> call any HCFS API-> stop counter -> log accordingly. This you can just hinge via fs.*.impl configs. Logging one API in one interface looks overkill to me. I won't block you if others are fine with it, but it won't help you much either IMO, |



What changes were proposed in this pull request?
This PR adds configurable WARN-level logging for slow getFileInfo calls performed during storage-based authorization checks.
A new configuration property,
hive.metastore.authorization.fileinfo.slow.warn.threshold.ms (default: 300 ms, -1 to disable), is introduced to control when a WARN log should be emitted.
StorageBasedAuthorizationProvider.checkPermissions() is instrumented to measure the time spent in FileUtils.getFileStatusOrNull() for both:
If the elapsed time exceeds the configured threshold, a WARN log containing the path and elapsed time is emitted.
Integration tests have also been added to verify:
Why are the changes needed?
Storage-based authorization relies on filesystem metadata lookups (getFileInfo), which may become slow due to NameNode latency or filesystem performance issues. These delays are currently not visible in normal logs, making them difficult to diagnose without enabling verbose logging or profiling.
This change provides lightweight observability by logging only operations whose latency exceeds a configurable threshold, helping administrators identify slow filesystem operations while avoiding excessive log noise.
Does this PR introduce any user-facing change?
Yes.
A new configuration property has been added:
hive.metastore.authorization.fileinfo.slow.warn.threshold.ms
When enabled, slow filesystem metadata lookups performed during storage-based authorization checks will generate WARN log entries if they exceed the configured threshold.
No functional behavior of authorization is changed.
How was this patch tested?
Added integration tests covering both positive and negative scenarios