Skip to content

HDDS-17240. Repair old volume quota without losing unflushed volume changes - #11846

Merged
adoroszlai merged 2 commits into
apache:masterfrom
rjgoyln:HDDS-17240
Oct 30, 2026
Merged

adoroszlai merged 2 commits into
apache:masterfrom
rjgoyln:HDDS-17240

Conversation

@rjgoyln

@rjgoyln rjgoyln commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

Full quota repair (ozone repair om quota start without --buckets, or the QUOTA upgrade action) converts volumes with the pre-1.1 quota value -2.

Previously, it read volumes from RocksDB and wrote them back to the fully cached volume table, potentially overwriting unflushed changes. This could restore deleted volumes, lose usage counter updates, overwrite recently updated quotas, or miss newly created volumes.

The response also wrote converted volumes under bare names instead of the proper volume table keys, causing the repair to be lost on restart and volumes to appear twice in listings.

This patch reads candidates from the volume table cache, rereads each volume under its lock before updating the quota, and writes the result using the proper volume table key.

Existing bare-name rows from previous repairs are not removed and may still appear in whole-table listings.

What is the link to the Apache JIRA?

https://issues.apache.org/jira/browse/HDDS-17240

How was this patch tested?

  • Added testOldVolumeQuotaRepairKeepsUnflushedVolumeChanges to cover unflushed deletions, usage counter changes, quota updates, and volume creation. All cases fail on master.
  • Extended testQuotaRepairForOldVersionVolumeBucket to verify the persisted quota and ensure no bare-name row is written.
  • Verified the JIRA reproductions pass with this patch and fail on master; control cases pass on both.
  • Quota repair, volume, tenant, and bucket request/response suites (150 tests) and TestOzoneRepairShell pass.

Generated by Claude Code (Opus 5.5).

…hanges

The repair found legacy volumes through the RocksDB iterator and put the
row it read back into the fully cached volume table. A volume delete, a
bucket count or tenant refCount change applied just before it but not yet
flushed was rolled back, and a volume created but not yet flushed kept its
old quota, so OMs diverged depending on how far their double buffer had
flushed. Its response also wrote each volume under the bare name, so the
conversion was lost on restart and the volume was listed twice.

Both are fixed together: correcting only the key would make the rolled
back value durable.
Copilot AI balanced review requested due to automatic review settings October 29, 2026 13:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@github-actions github-actions Bot added the om label Oct 29, 2026
@rjgoyln
rjgoyln marked this pull request as ready for review October 29, 2026 18:07

@smengcl smengcl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @rjgoyln for the patch.

for (OmVolumeArgs volArgs : volumeArgsMap.values()) {
metadataManager.getVolumeTable().putWithBatch(batchOp, volArgs.getVolume(), volArgs);
metadataManager.getVolumeTable().putWithBatch(batchOp,
metadataManager.getVolumeKey(volArgs.getVolume()), volArgs);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(non-blocking) Follow-up: the bare-name rows written by earlier repairs stay in RocksDB and are loaded into the volume table cache on restart. listAllVolumes iterates that cache, so an admin ozone sh volume list shows each repaired volume twice, and VolumeUtilizationMetrics emits two records for it. volume delete removes only the /vol key, so the bare row remains after the volume is deleted.

Could we file a follow-up JIRA to remove these rows (for example, a row whose key has no / prefix and equals its getVolume())?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the verifying! I will file a new Jira ticket for it.

@chungen0126 chungen0126 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 LGTM

@adoroszlai
adoroszlai merged commit 28c8514 into apache:master Oct 30, 2026
131 of 135 checks passed
@adoroszlai

adoroszlai commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Thanks @rjgoyln for the patch, @chungen0126, @smengcl for the review.

For future PRs please add Co-authored-by in the commit message instead of Generated-by in the PR description (see HDDS-17108 for details). No need to update PRs already opened.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants