Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HDDS-10425. Increase OM transaction index for non-Ratis based on existing Ratis transactionInfoTable #6281

Merged
merged 2 commits into from
Feb 28, 2024

Conversation

whbing
Copy link
Contributor

@whbing whbing commented Feb 27, 2024

What changes were proposed in this pull request?

ObjectIDs duplicate when non-Ratis OM start from an existing Ratis transactionInfoTable.
Transaction index should increase incrementally based on the existing transactionInfoTable for non-Ratis rather than from 0.
This approach will facilitate the migration from Ratis to non-Ratis scenarios seamlessly.

What is the link to the Apache JIRA

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

How was this patch tested?

manual tests.
For origin ratis transactionInfoTable

bin/ozone debug ldb --db=/home/hadoop/cluster-data/ozonedata/om/db/om.db scan --cf=transactionInfoTable

{ "#TRANSACTIONINFO": {
"term" : 1,
"transactionIndex" : 9
}

(1) without pr
migrate to non-ratis

bin/ozone debug ldb --db=/home/hadoop/cluster-data/ozonedata/om/db/om.db scan --cf=transactionInfoTable

{ "#TRANSACTIONINFO": {
  "term" : -1,
  "transactionIndex" : 3
}

(2) with pr
migrate to non-ratis

bin/ozone debug ldb --db=/home/hadoop/cluster-data/ozonedata/om/db/om.db scan --cf=transactionInfoTable
{ "#TRANSACTIONINFO": {
  "term" : -1,
  "transactionIndex" : 11
}

TransactionIndex 11 increases from 9 is what we expect.

Increase OM transaction index for non-Ratis based on existing Ratis transactionInfoTable
@whbing
Copy link
Contributor Author

whbing commented Feb 27, 2024

@ChenSammi @hanishakoneru Could you help review if you have the time ? Thanks !

Copy link
Contributor

@Galsza Galsza left a comment

Choose a reason for hiding this comment

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

Thanks @whbing for working on this, I've added a minor nitpick for this change, you can ignore or fix it.

Is it possible to also add a test for this case?

Copy link
Contributor

@sumitagrawl sumitagrawl left a comment

Choose a reason for hiding this comment

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

LGTM +1

Copy link
Contributor

@ChenSammi ChenSammi left a comment

Choose a reason for hiding this comment

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

Thanks @whbing , and @Galsza @sumitagrawl for the review.

@Galsza
Copy link
Contributor

Galsza commented Feb 28, 2024

@whbing thanks for the patch, LGTM +1

@ChenSammi ChenSammi merged commit 4da5a64 into apache:master Feb 28, 2024
35 checks passed
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Mar 15, 2024
…ting Ratis transactionInfoTable (apache#6281)

(cherry picked from commit 4da5a64)
Change-Id: Ibdad3f657cb5b481c1ec2655473b9df48011d8c1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants