You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2025/09/03 19:43:19 UTC
[PR] HDDS-13633. Snapshot Cache loader should check if the double buffer has flushed the create transaction [ozone]
swamirishi opened a new pull request, #8999:
URL: https://github.com/apache/ozone/pull/8999
## What changes were proposed in this pull request?
Right now Snapshot Cache while trying to load snapshot checks if the directory merely exists however since we are going to create another snapshot local metadata file and this is going to be required for the snapshot diff. Thus it would be a better model if we have create txn id similar to lastTxnId in snapshotInfo and check if the txn id has been flushed.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13633
## How was this patch tested?
Additional unit tests added
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13633. Snapshot Cache loader should check if the double buffer has flushed the create transaction [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #8999:
URL: https://github.com/apache/ozone/pull/8999#discussion_r2320276975
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -397,9 +394,29 @@ public OmSnapshot load(@Nonnull UUID snapshotId) throws IOException {
// on that.
OMMetadataManager snapshotMetadataManager;
try {
+ // The check is only to prevent every snapshot read to perform a disk IO
+ // and check if a checkpoint dir exists. If entry is present in cache,
+ // it is most likely DB entries will get flushed in this wait time.
+ Duration maxPollDuration =
+ Duration.ofMillis(conf.getTimeDuration(
+ OZONE_SNAPSHOT_CHECKPOINT_DIR_CREATION_POLL_TIMEOUT,
+ OZONE_SNAPSHOT_CHECKPOINT_DIR_CREATION_POLL_TIMEOUT_DEFAULT,
+ TimeUnit.MILLISECONDS));
+ boolean flushed = RatisHelper.attemptUntilTrue(() -> {
+ try {
+ return OmSnapshotManager.isSnapshotFlushedToDB(ozoneManager.getMetadataManager(), snapshotInfo);
+ } catch (IOException e) {
+ return false;
+ }
+ }, POLL_INTERVAL_DURATION, maxPollDuration);
+ if (!flushed) {
+ throw new OMException("Unable to load snapshot. " +
+ "Create Snapshot Txn '" + snapshotInfo.getTableKey() +
Review Comment:
maybe it should log snapshotInfo.getCreateTransactionInfo() instead?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13633. Snapshot Cache loader should check if the double buffer has flushed the create transaction [ozone]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8999:
URL: https://github.com/apache/ozone/pull/8999#discussion_r2320040093
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -397,9 +394,29 @@ public OmSnapshot load(@Nonnull UUID snapshotId) throws IOException {
// on that.
OMMetadataManager snapshotMetadataManager;
try {
+ // The check is only to prevent every snapshot read to perform a disk IO
+ // and check if a checkpoint dir exists. If entry is present in cache,
+ // it is most likely DB entries will get flushed in this wait time.
+ Duration maxPollDuration =
+ Duration.ofMillis(conf.getTimeDuration(
+ OZONE_SNAPSHOT_CHECKPOINT_DIR_CREATION_POLL_TIMEOUT,
+ OZONE_SNAPSHOT_CHECKPOINT_DIR_CREATION_POLL_TIMEOUT_DEFAULT,
+ TimeUnit.MILLISECONDS));
+ boolean flushed = RatisHelper.attemptUntilTrue(() -> {
+ try {
+ return OmSnapshotManager.isSnapshotFlushedToDB(ozoneManager.getMetadataManager(), snapshotInfo);
+ } catch (IOException e) {
+ return false;
+ }
+ }, POLL_INTERVAL_DURATION, maxPollDuration);
+ if (!flushed) {
+ throw new OMException("Unable to load snapshot. " +
+ "Create Snapshot Txn '" + snapshotInfo.getTableKey() +
Review Comment:
The error message includes `snapshotInfo.getTableKey()` but this method may not exist on SnapshotInfo. Consider using a more appropriate identifier like the snapshot ID or name for better clarity.
```suggestion
"Snapshot ID '" + snapshotInfo.getSnapshotId() +
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13633. Snapshot Cache loader should check if the double buffer has flushed the create transaction [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #8999:
URL: https://github.com/apache/ozone/pull/8999#issuecomment-3250597271
### What does the PR do?
**Current behavior:** When loading a snapshot in Ozone, the Snapshot Cache checks if the directory for the snapshot exists. That’s its main criterion to proceed.
**Problem:** Just checking for the existence of the directory is not enough. Snapshots rely on metadata files that are created as part of the snapshot process, and these files are used for operations like snapshot diff. If the snapshot creation transaction hasn’t been completely flushed (i.e., written and persisted), we might end up reading incomplete or inconsistent snapshot metadata.
**Proposed change:** The PR adds a check for the “create transaction ID” (similar to how “lastTxnId” is tracked in snapshotInfo). It verifies if the double buffer (the mechanism that batches and flushes write operations) has flushed the transaction that created the snapshot. Only if the transaction is flushed and persisted does the loader proceed to use the snapshot cache.
### What does this mean in the code?
* The diff adds logic to store and check a transaction ID associated with snapshot creation.
* When loading a snapshot, instead of just looking for the directory, it now looks for both: existence of the directory and confirmation that the transaction which created it has been flushed.
* This helps guarantee that all required metadata files (especially those used for snapshot diff) are present and complete.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13633. Snapshot Cache loader should check if the double buffer has flushed the create transaction [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #8999:
URL: https://github.com/apache/ozone/pull/8999#issuecomment-3255619486
Merged. Thanks @swamirishi
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13633. Snapshot Cache loader should check if the double buffer has flushed the create transaction [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #8999:
URL: https://github.com/apache/ozone/pull/8999#discussion_r2321435272
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -397,9 +394,29 @@ public OmSnapshot load(@Nonnull UUID snapshotId) throws IOException {
// on that.
OMMetadataManager snapshotMetadataManager;
try {
+ // The check is only to prevent every snapshot read to perform a disk IO
+ // and check if a checkpoint dir exists. If entry is present in cache,
+ // it is most likely DB entries will get flushed in this wait time.
+ Duration maxPollDuration =
+ Duration.ofMillis(conf.getTimeDuration(
+ OZONE_SNAPSHOT_CHECKPOINT_DIR_CREATION_POLL_TIMEOUT,
+ OZONE_SNAPSHOT_CHECKPOINT_DIR_CREATION_POLL_TIMEOUT_DEFAULT,
+ TimeUnit.MILLISECONDS));
+ boolean flushed = RatisHelper.attemptUntilTrue(() -> {
+ try {
+ return OmSnapshotManager.isSnapshotFlushedToDB(ozoneManager.getMetadataManager(), snapshotInfo);
+ } catch (IOException e) {
+ return false;
+ }
+ }, POLL_INTERVAL_DURATION, maxPollDuration);
+ if (!flushed) {
+ throw new OMException("Unable to load snapshot. " +
+ "Create Snapshot Txn '" + snapshotInfo.getTableKey() +
Review Comment:
Oh let me add the transactionInfo as well in the log. However I wanted to just print the statement saying we have not flushed the createSnapshot txn yet.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13633. Snapshot Cache loader should check if the double buffer has flushed the create transaction [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang merged PR #8999:
URL: https://github.com/apache/ozone/pull/8999
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org