HDDS-11901. use om managed index for handling request by sumitagrawl · Pull Request #7722 · apache/ozone

sumitagrawl · 2025-01-19T13:46:00Z

What changes were proposed in this pull request?

Remove dependency with ratis index for OM functionality for request object. Introduced IndexManager that will keep track of OM index and provide next index for request execution.

Make use of OM managed index as IndexManager class which will generate index, save commit index, on leader change action to update index
Persist Index in TransactionInfoTable as #OMINDEX during doubleBuffer commit operation
Support OMLayout Feature flag for handling upgrade and reinit on finalize

Flow:
ClientRequest: --> OmExecutionFlow --> generate index and add to Request ---> submit to ratis
Ratis: ApplyTransaction --> retrieve index and update ExecutionContext --> perform operation

Usages:

It will simplify applying om db from production environment to local for debug purpose, as ratis index impact will not cause failure to start.
Will be used for pre-ratis execution feature

Refer (HDDS-11898. design doc leader side execution) for Index generation and Step-by-step incremental changes for existing flow

Parent Jira:
https://issues.apache.org/jira/browse/HDDS-11897

What is the link to the Apache JIRA

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

How was this patch tested?

Unit test case
Existing integration test for any impact

adoroszlai · 2025-02-17T14:55:56Z

Sorry for cancelling the workflow. I will resolve conflicts after HDDS-12277 and trigger CI.

adoroszlai

Thanks @sumitagrawl for the patch.

sumitagrawl · 2025-02-19T07:30:02Z

@adoroszlai resolved all comments and fixed license header changes, please review again

adoroszlai · 2025-02-19T07:33:38Z

fixed license header changes

I have already fixed license headers, see:

adoroszlai · 2025-02-19T17:11:30Z

Thanks @sumitagrawl for addressing my comments. I'll let others review as well.

ChenSammi · 2025-02-24T09:32:32Z

+ }
+ index.set(initIndex);
+ commitIndex.set(initIndex);
+ if (ozoneManager.getVersionManager().needsFinalization()) {


Should check whether this specific feature needs finalization, instead of a general needsFinalization check, as needsFinalization will return true once there is new feature added and not finalized.

ChenSammi · 2025-02-24T11:10:51Z

 LOG.info("{}: leader changed to {}", groupMemberId, newLeaderId);
+ // if the node is leader (can be ready or not ready, need update index)
+ if (ozoneManager.getOmRatisServer().checkLeaderStatus() != OzoneManagerRatisServer.RaftServerStatus.NOT_LEADER) {
+ indexGenerator.onLeaderChange();


As every OM has its IndexGenerator, which works independently, could you explain a little bit here, what the onLeaderChange actually do?

Since every OM has its IndexGenerator which works independently, could it be possible that for the same TermIndex transaction, its index has different value on different OM? If this happens, what will be the consequence?

Ratis generate termIndex for below cases:
transaction update via applyTransaction()
notifyConfigChange for case of ratis internal book-keeping
IndexGenerate will generate index in pre-ratis execution for every incoming request at leader node.
So both can have different value, where mostly ratis index will be faster side.
OM will use OM index for purpose of objectId and updateId generation as used currently.
TermIndex will be used to handle replay of ratis log pending, and ensure nodes are in sync in upgrade prepare cases.
So use case will be separated and hence will not have impact.

@ChenSammi I got the problem,
Since every node generate the index on request execution, so there is a possibility of different index value. All nodes should get index from common place in this case, will work over this to handle.

sumitagrawl · 2025-02-25T09:24:59Z

@ChenSammi updated the comments and code, plz review again

ChenSammi · 2025-03-03T10:50:02Z

@ChenSammi updated the comments and code, plz review again

Per we offline discussion, I suggest to start a feature branch for all these pre-ratis execution changes, as the idea of keeping both pre-ratis execution and current per-ratis execution work is really a ideal state. But mixing the code makes it complex, hard to understand clearly, and easy to go wrong on many edge cases, for example, this new Index should not the turned on until only the OM leader will call the IndexGenerator#nextIndex(). A feature branch will save the master from broken due to intermediate development state, cases not covered, bugs, etc.

sumitagrawl · 2025-03-20T13:22:52Z

@ChenSammi Please review the changes done for fix

Since every OM has its IndexGenerator which works independently, could it be possible that for the same TermIndex transaction, its index has different value on different OM? If this happens, what will be the consequence?

As change, ExecutionControlRequest parameter is added, and leader will append index generated before submit the request. This will resolve the above problem.

adoroszlai · 2025-04-03T10:55:26Z

Should this be targeted at the feature branch?

sumitagrawl added the om-pre-ratis-execution PRs related to https://issues.apache.org/jira/browse/HDDS-11897 label Jan 19, 2025

sumitagrawl requested review from ChenSammi, aswinshakil, errose28, kerneltime, nandakumar131 and swamirishi January 22, 2025 05:34

sumitagrawl force-pushed the HDDS-11901 branch from d814294 to a12079fCompare February 17, 2025 06:04

sumitagrawl marked this pull request as ready for review February 17, 2025 14:46

adoroszlai marked this pull request as draft February 17, 2025 14:54

adoroszlai self-assigned this Feb 17, 2025

adoroszlai reviewed Feb 17, 2025
View reviewed changes

Comment thread...one/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/execution/common/BiConsumerX.java Outdated

adoroszlai reviewed Feb 17, 2025
View reviewed changes

adoroszlai removed their assignment Feb 17, 2025

sumitagrawl force-pushed the HDDS-11901 branch from 7439a51 to 0d3a31dCompare February 19, 2025 07:26

sumitagrawl requested a review from adoroszlai February 19, 2025 07:29

sumitagrawl marked this pull request as ready for review February 19, 2025 16:16

ChenSammi reviewed Feb 24, 2025
View reviewed changes

Comment thread...manager/src/main/java/org/apache/hadoop/ozone/om/execution/flowcontrol/ExecutionContext.java Outdated

ChenSammi reviewed Feb 24, 2025
View reviewed changes

Comment thread...manager/src/main/java/org/apache/hadoop/ozone/om/execution/flowcontrol/ExecutionContext.java

ChenSammi reviewed Feb 24, 2025
View reviewed changes

Comment thread...p-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/execution/IndexGenerator.java

ChenSammi reviewed Feb 24, 2025
View reviewed changes

sumitagrawl requested a review from ChenSammi February 25, 2025 09:24

sumitagrawl mentioned this pull request Mar 19, 2025
HDDS-12549. refactor ratis request to common place #8059
Merged

sumitagrawl marked this pull request as draft March 20, 2025 05:56

sumitagrawl added 10 commits March 20, 2025 13:16

HDDS-11901. use om managed index for handling request
68e94ea

support upgrade handling for index change
540f0c6

fix checkstyle and add test case
91651e5

fix test failure
2eb709d

test case failure fix
b2d9865

fix test case
fb54f57

fix test case
bcee077

review comment fix
4b2916d

fix checkstyle
a101418

fix review comments
9a65251

sumitagrawl force-pushed the HDDS-11901 branch from ba965a7 to 9a65251Compare March 20, 2025 07:46

fix index generation at leader
8e6e8e9

adoroszlai closed this Jun 5, 2025

Uh oh!

Conversation

sumitagrawl commented Jan 19, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

adoroszlai commented Feb 17, 2025

Uh oh!

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sumitagrawl commented Feb 19, 2025

Uh oh!

adoroszlai commented Feb 19, 2025

Uh oh!

adoroszlai commented Feb 19, 2025

Uh oh!

ChenSammiFeb 24, 2025

Choose a reason for hiding this comment

Uh oh!

sumitagrawlFeb 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ChenSammiFeb 24, 2025

Choose a reason for hiding this comment

Uh oh!

ChenSammiFeb 24, 2025

Choose a reason for hiding this comment

Uh oh!

sumitagrawlFeb 25, 2025

Choose a reason for hiding this comment

Uh oh!

sumitagrawlFeb 28, 2025

Choose a reason for hiding this comment

Uh oh!

sumitagrawl commented Feb 25, 2025

Uh oh!

ChenSammi commented Mar 3, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sumitagrawl commented Mar 20, 2025

Uh oh!

adoroszlai commented Apr 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sumitagrawl commented Jan 19, 2025•
edited
Loading

ChenSammi commented Mar 3, 2025•
edited
Loading