[ 
https://issues.apache.org/jira/browse/HDDS-16175?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Chi-Hsuan Huang updated HDDS-16175:
-----------------------------------
    Description: 
### Problem

A bucket's {{usedNamespace}} is charged once per key commit but refunded only 
once per key.
Every hsync re-commit of the same key by the same client charges the counter 
again without
ever adding a key, so a file that is hsync'd N times consumes N+1 namespace 
units while
holding one name. Deleting the key refunds 1. The residue is permanent: after 
every key in
the bucket has been deleted, {{usedNamespace}} stays at a non-zero value and 
never returns
to 0. It permanently consumes the bucket's namespace quota, so
{{OMKeyRequest.checkBucketQuotaInNamespace}} eventually rejects writes on a 
bucket that
holds fewer keys than its quota allows. {{ozone sh bucket info}} and the Recon 
quota bar
read the same persisted counter.

This affects any hsync workload, HBase WALs in particular, on both OBS and FSO 
buckets.

### Root cause

{{c351de9914}} (HDDS-13756, "Introduce Bucket Snapshot Used Bytes and 
SnapshotUsedNamespace
in BucketInfo") hoisted {{incrUsedNamespace(1L)}} out of the {{else}} branch 
and placed it
after the whole if/else chain:

    } else {
      checkBucketQuotaInNamespace(omBucketInfo, 1L);
      checkBucketQuotaInBytes(omMetadataManager, omBucketInfo, correctedSpace);
-     omBucketInfo.incrUsedNamespace(1L);
    }
-
+   omBucketInfo.incrUsedNamespace(1L);

Every commit now increments the counter, including the {{isSameHsyncKey}} 
branch at
{{OMKeyCommitRequest.java:324}}, which handles a re-commit of a key the same 
client already
hsync'd. That branch previously did not touch namespace, and that was 
deliberate:
{{df4df20371}} (HDDS-7965, "Quota needs to be updated correctly for Hsync") 
introduced it
precisely to settle only the byte delta for an hsync re-commit:

    -      if (keyToDelete != null && !omBucketInfo.getIsVersionEnabled()) {
    +      if (keyToDelete != null && isHSync) {
    +        correctedSpace -= keyToDelete.getReplicatedSize();
    +        checkBucketQuotaInBytes(omMetadataManager, omBucketInfo, 
correctedSpace);
    +      } else if (keyToDelete != null && 
!omBucketInfo.getIsVersionEnabled()) {

({{d7e5b3a3fd}}, HDDS-10770, later narrowed the condition from {{isHSync}} to
{{isSameHsyncKey}}; the intent was unchanged.) HDDS-13756 therefore reverts 
HDDS-7965 for
the namespace counter while leaving its byte handling in place.

The refund side is unchanged: {{OMKeyDeleteRequest.java:169}} refunds a flat
{{decrUsedNamespace(1L, ...)}} per key, and the neighbouring line 168 releases 
bytes via
{{sumBlockLengths}}. Bytes are therefore symmetric while namespace is not.

Both commit paths carry the hoisted increment:
* {{OMKeyCommitRequest.java:375}}
* {{OMKeyCommitRequestWithFSO.java:307}}

### Expected semantics

Every other surface treats {{usedNamespace}} as a count of keys:

* {{a58d3f5a00}} (HDDS-4277) introduced the counter with the commit body "add
  namespaceQuotaUsage and update it when create and delete key in a bucket".
* {{hadoop-hdds/docs/content/feature/Quota.md:57}}: "When bucket namespace 
quota is enabled,
  the total number of keys under the bucket, cannot exceed the bucket namespace 
quota", and
  "Namespace quota is a number that represents how many unique names can be 
used".
* All six refund sites ({{OMKeyDeleteRequest}}, {{OMKeyDeleteRequestWithFSO}},
  {{OMKeysDeleteRequest}}, {{OMDirectoriesPurgeRequestWithFSO}}, 
{{OMKeyPurgeRequest}}, and
  the overwrite branch of {{OMKeyCommitRequest}}) refund one unit per key.
* {{OzoneRpcClientTests.java:1921}}: "Test create a file twice will not increase
  usedNamespace twice".

No commit message, javadoc, document or test in the repository states that 
re-committing a
key should consume an additional namespace unit.

### Fix

Charge {{incrUsedNamespace(1L)}} only on the paths that actually add a key, 
i.e. move it
back inside the branches rather than after the chain, in both 
{{OMKeyCommitRequest}} and
{{OMKeyCommitRequestWithFSO}}.

The regression shipped unnoticed because no test asserts {{usedNamespace}} 
across repeated
hsync commits; the quota tests around this code assert {{getUsedBytes()}} only. 
A fix should
add an assertion that hsync'ing a key N times leaves {{usedNamespace}} at 1, 
and that the
counter returns to 0 after the key is deleted.

### Note on bucket versioning

The same hoisted increment also charges once per version on a 
versioning-enabled bucket,
because {{ff351025bc}} (HDDS-6709) routed versioned overwrites into the 
{{else}} branch that
carries the increment. That half is out of scope here: per the discussion on 
HDDS-16127,
there is no code path that exercises bucket versioning today, and HDDS-15728 
object
versioning stores each version as a separate {{OmKeyInfo}}. This issue is 
limited to the
hsync path, which is exercised by ordinary non-versioned buckets.

The stale comment at {{OMKeyCommitRequest.java:322-323}}, "if keyToDelete isn't 
null,
usedNamespace needn't check and increase" (introduced by {{9055a11216}}, 
HDDS-6556), still
contradicts the code and can be corrected alongside the fix.

### Notes

Found while scoping HDDS-16127. Pinned source commit 4766aa8609. Analysis 
assisted by AI
tooling (Claude Code, Opus 5).


  was:
h3. Problem
A bucket's {{usedNamespace}} is charged once per committed key version but 
refunded only once per key. After every key in a versioning\-enabled bucket has 
been deleted, {{usedNamespace}} stays at a non\-zero value and never returns to 
0. The residue permanently consumes the bucket's namespace quota, so 
{{OMKeyRequest.checkBucketQuotaInNamespace}} eventually rejects writes on a 
bucket that holds fewer keys than its quota allows.

Repeated {{hsync}} commits of the same key charge the counter the same way, 
without ever adding a key.

h3. Root cause
{{OMKeyCommitRequest.java:375}} calls {{omBucketInfo.incrUsedNamespace\(1L\)}} 
outside the if/else chain, so every commit increments it, including each new 
version of an existing key on a versioning\-enabled bucket and each re\-commit 
of an hsync'd key. {{OMKeyDeleteRequest.java:169}} refunds a flat 
{{decrUsedNamespace\(1L, ...\)}} regardless of how many versions the key holds, 
and the neighbouring line 165 releases bytes for all versions via 
{{sumBlockLengths}}. Bytes are therefore symmetric while namespace is not.

The per\-version charge was not intended. {{ff351025bc}} \(HDDS\-6709, "Fix 
bucket usedBytes while versioning is true"\) changed only the branch condition:

{code}
\-      if \(keyToDelete \!= null\) {
\+      if \(keyToDelete \!= null && \!omBucketInfo.getIsVersionEnabled\(\)\) {
{code}

which routed every commit on a versioning\-enabled bucket into the {{else}} 
branch that carries the namespace increment. That commit's test, 
{{bucketUsedBytesOverWrite}}, asserts {{getUsedBytes\(\)}} only and makes no 
assertion about {{usedNamespace}}. The comment introduced by {{9055a11216}} 
\(HDDS\-6556\), "if keyToDelete isn't null, usedNamespace needn't check and 
increase", was left in place and still contradicts the code.

The hsync case is a second, later regression: {{c351de9914}} \(HDDS\-13756\) 
hoisted {{incrUsedNamespace\(1L\)}} fully outside the if/else, so the 
{{isSameHsyncKey}} branch at lines 324\-327, which previously did not charge 
namespace, now charges on every re\-commit.

h3. Expected semantics
Every other surface treats {{usedNamespace}} as a count of keys:

* {{a58d3f5a00}} \(HDDS\-4277\) introduced the counter with the commit body 
"add namespaceQuotaUsage and update it when create and delete key in a bucket".
* {{hadoop\-hdds/docs/content/feature/Quota.md:57}}: "When bucket namespace 
quota is enabled, the total number of keys under the bucket, cannot exceed the 
bucket namespace quota", and "Namespace quota is a number that represents how 
many unique names can be used".
* All six refund sites \({{OMKeyDeleteRequest}}, {{OMKeyDeleteRequestWithFSO}}, 
{{OMKeysDeleteRequest}}, {{OMDirectoriesPurgeRequestWithFSO}}, 
{{OMKeyPurgeRequest}}, and the non\-versioned overwrite branch of 
{{OMKeyCommitRequest}}\) refund one unit per key.
* The Recon UI renders it as a used/total quota bar, and {{ozone sh bucket 
info}} prints it next to {{quotaInNamespace}}.
* {{OzoneRpcClientTests.java:1921}}: "Test create a file twice will not 
increase usedNamespace twice".

No commit message, javadoc, document or test in the repository states that a 
multi\-version key should consume more than one namespace unit.

h3. Impact
{{usedNamespace}} drifts upward and never recovers on buckets that use 
versioning or hsync. Namespace quota enforcement and {{ozone sh bucket info}} 
both read the persisted counter.

h3. Notes
Found while scoping HDDS\-16127, which fixes the corresponding {{usedBytes}} 
undercount in quota repair and deliberately leaves {{usedNamespace}} unchanged, 
because quota repair currently counts one unit per key and therefore corrects 
this drift rather than reproducing it.

Pinned source commit 4766aa8609. Analysis assisted by AI tooling \(Claude Code, 
Opus 5\).



        Summary: usedNamespace is charged on every hsync re-commit of the same 
key  (was: usedNamespace is charged per key version but refunded per key)

> usedNamespace is charged on every hsync re-commit of the same key
> -----------------------------------------------------------------
>
>                 Key: HDDS-16175
>                 URL: https://issues.apache.org/jira/browse/HDDS-16175
>             Project: Apache Ozone
>          Issue Type: Bug
>          Components: OM
>            Reporter: Chi-Hsuan Huang
>            Priority: Major
>
> ### Problem
> A bucket's {{usedNamespace}} is charged once per key commit but refunded only 
> once per key.
> Every hsync re-commit of the same key by the same client charges the counter 
> again without
> ever adding a key, so a file that is hsync'd N times consumes N+1 namespace 
> units while
> holding one name. Deleting the key refunds 1. The residue is permanent: after 
> every key in
> the bucket has been deleted, {{usedNamespace}} stays at a non-zero value and 
> never returns
> to 0. It permanently consumes the bucket's namespace quota, so
> {{OMKeyRequest.checkBucketQuotaInNamespace}} eventually rejects writes on a 
> bucket that
> holds fewer keys than its quota allows. {{ozone sh bucket info}} and the 
> Recon quota bar
> read the same persisted counter.
> This affects any hsync workload, HBase WALs in particular, on both OBS and 
> FSO buckets.
> ### Root cause
> {{c351de9914}} (HDDS-13756, "Introduce Bucket Snapshot Used Bytes and 
> SnapshotUsedNamespace
> in BucketInfo") hoisted {{incrUsedNamespace(1L)}} out of the {{else}} branch 
> and placed it
> after the whole if/else chain:
>     } else {
>       checkBucketQuotaInNamespace(omBucketInfo, 1L);
>       checkBucketQuotaInBytes(omMetadataManager, omBucketInfo, 
> correctedSpace);
> -     omBucketInfo.incrUsedNamespace(1L);
>     }
> -
> +   omBucketInfo.incrUsedNamespace(1L);
> Every commit now increments the counter, including the {{isSameHsyncKey}} 
> branch at
> {{OMKeyCommitRequest.java:324}}, which handles a re-commit of a key the same 
> client already
> hsync'd. That branch previously did not touch namespace, and that was 
> deliberate:
> {{df4df20371}} (HDDS-7965, "Quota needs to be updated correctly for Hsync") 
> introduced it
> precisely to settle only the byte delta for an hsync re-commit:
>     -      if (keyToDelete != null && !omBucketInfo.getIsVersionEnabled()) {
>     +      if (keyToDelete != null && isHSync) {
>     +        correctedSpace -= keyToDelete.getReplicatedSize();
>     +        checkBucketQuotaInBytes(omMetadataManager, omBucketInfo, 
> correctedSpace);
>     +      } else if (keyToDelete != null && 
> !omBucketInfo.getIsVersionEnabled()) {
> ({{d7e5b3a3fd}}, HDDS-10770, later narrowed the condition from {{isHSync}} to
> {{isSameHsyncKey}}; the intent was unchanged.) HDDS-13756 therefore reverts 
> HDDS-7965 for
> the namespace counter while leaving its byte handling in place.
> The refund side is unchanged: {{OMKeyDeleteRequest.java:169}} refunds a flat
> {{decrUsedNamespace(1L, ...)}} per key, and the neighbouring line 168 
> releases bytes via
> {{sumBlockLengths}}. Bytes are therefore symmetric while namespace is not.
> Both commit paths carry the hoisted increment:
> * {{OMKeyCommitRequest.java:375}}
> * {{OMKeyCommitRequestWithFSO.java:307}}
> ### Expected semantics
> Every other surface treats {{usedNamespace}} as a count of keys:
> * {{a58d3f5a00}} (HDDS-4277) introduced the counter with the commit body "add
>   namespaceQuotaUsage and update it when create and delete key in a bucket".
> * {{hadoop-hdds/docs/content/feature/Quota.md:57}}: "When bucket namespace 
> quota is enabled,
>   the total number of keys under the bucket, cannot exceed the bucket 
> namespace quota", and
>   "Namespace quota is a number that represents how many unique names can be 
> used".
> * All six refund sites ({{OMKeyDeleteRequest}}, {{OMKeyDeleteRequestWithFSO}},
>   {{OMKeysDeleteRequest}}, {{OMDirectoriesPurgeRequestWithFSO}}, 
> {{OMKeyPurgeRequest}}, and
>   the overwrite branch of {{OMKeyCommitRequest}}) refund one unit per key.
> * {{OzoneRpcClientTests.java:1921}}: "Test create a file twice will not 
> increase
>   usedNamespace twice".
> No commit message, javadoc, document or test in the repository states that 
> re-committing a
> key should consume an additional namespace unit.
> ### Fix
> Charge {{incrUsedNamespace(1L)}} only on the paths that actually add a key, 
> i.e. move it
> back inside the branches rather than after the chain, in both 
> {{OMKeyCommitRequest}} and
> {{OMKeyCommitRequestWithFSO}}.
> The regression shipped unnoticed because no test asserts {{usedNamespace}} 
> across repeated
> hsync commits; the quota tests around this code assert {{getUsedBytes()}} 
> only. A fix should
> add an assertion that hsync'ing a key N times leaves {{usedNamespace}} at 1, 
> and that the
> counter returns to 0 after the key is deleted.
> ### Note on bucket versioning
> The same hoisted increment also charges once per version on a 
> versioning-enabled bucket,
> because {{ff351025bc}} (HDDS-6709) routed versioned overwrites into the 
> {{else}} branch that
> carries the increment. That half is out of scope here: per the discussion on 
> HDDS-16127,
> there is no code path that exercises bucket versioning today, and HDDS-15728 
> object
> versioning stores each version as a separate {{OmKeyInfo}}. This issue is 
> limited to the
> hsync path, which is exercised by ordinary non-versioned buckets.
> The stale comment at {{OMKeyCommitRequest.java:322-323}}, "if keyToDelete 
> isn't null,
> usedNamespace needn't check and increase" (introduced by {{9055a11216}}, 
> HDDS-6556), still
> contradicts the code and can be corrected alongside the fix.
> ### Notes
> Found while scoping HDDS-16127. Pinned source commit 4766aa8609. Analysis 
> assisted by AI
> tooling (Claude Code, Opus 5).



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to