henry3260 commented on code in PR #11157:
URL: https://github.com/apache/ozone/pull/11157#discussion_r3889929021
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java:
##########
@@ -685,6 +764,7 @@ public Response head(
addLastModifiedDate(response, key);
addTagCountIfAny(response, key);
addCustomMetadataHeaders(response, key);
+ addExpirationHeader(response, bucketName, keyPath, key);
Review Comment:
> `addExpirationHeader` runs on every successful HEAD and makes a fresh
`getLifecycleConfiguration` RPC that takes the bucket read lock. For a bucket
with no lifecycle config (the common case) OM throws
`LIFECYCLE_CONFIGURATION_NOT_FOUND`, logs it at ERROR with a stack trace
(`OmMetadataManagerImpl#getLifecycleConfiguration`), and records an audit READ
FAILURE, none of which the gateway `catch` can suppress. HEAD is a hot path, so
this floods the OM and audit logs.
>
> Could we gate the lookup on the default path, e.g. cache the config in S3G
or check a cheap "has lifecycle" flag on the bucket info HEAD already reads?
Agreed those happen in OM, so the gateway's catch can't suppress them.
While checking I found the not-found case is already logged at DEBUG right
before the throw; the ERROR-with-stack-trace comes from a catch-all on
IOException below it:
https://github.com/apache/ozone/blob/a17ad8798a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java#L1806-L1818
So NOT_FOUND surfacing as an ERROR plus an audit failure is pre-existing,
but latent: the only caller today is GetBucketLifecycleConfiguration, which
isn't a hot path. This PR is what makes it hot.
It also means a cache alone won't fix it -- with a TTL, the first HEAD per
bucket
per interval still logs an ERROR. So I'll split it: the OM side (not
treating NOT_FOUND as an error or an audit failure) goes in its own Jira since
it's OM error handling and can land independently. I'll add the negative cache
here so repeated HEADs on unconfigured buckets skip the RPC.
(HEAD doesn't read bucket info today, and OmBucketInfo has no lifecycle
field, so
a flag there would need a protobuf change.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
