HDDS-6743. Specify leader node for OM failover #3409

symious · 2022-05-13T07:26:46Z

What changes were proposed in this pull request?

Currently if clients first connect to a follower OM, the response show the OM is not leader but didn't specify the real Leader node.

This ticket is to let the reply to contains the Leader OM so that clients can connect to Leader node more conveniently.

What is the link to the Apache JIRA

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

How was this patch tested?

unit test

symious · 2022-05-13T07:27:41Z

@adoroszlai @ChenSammi Could you help to review this PR?

JacksonYao287 · 2022-05-13T10:14:14Z

thanks @symious for the work!
i have a patch for this issue earlier #2765

@hanishakoneru left a comment to explain why this should not be done. #2765 (comment)

i suggest we should achieve agreement on this issue first , and then go ahead.

symious · 2022-05-13T11:27:50Z

@JacksonYao287 Sure, thanks for the review.

In #2765 (comment), the concern I think is the misconfig of client side might trigger some dead loops, so an address was prefered to add instead of only OMNodeId.

In the latest commit of this PR, the OMNotLeaderException includes the following information:

raftPeerId
raftLeaderId
raftLeaderAddress

An example of this exception message would be
org.apache.hadoop.ozone.om.exceptions.OMNotLeaderException: OM:omNode-3 is not the leader. Suggested leader is OM:omNode-1/127.0.0.1
, when client received this exception, he should try the address first, only if the address is empty should he try to check the raftLeaderId we suggested.

kerneltime · 2022-10-31T16:35:01Z

@symious is this PR still active? If not we can close it.

DaveTeng0 · 2022-11-01T05:10:15Z

Just saw this PR, recently I've also been researching some issue related to the out-of-sync mapping between client and server. just mark myself here in order to follow up the latest change of this PR! thanks all!

symious · 2022-11-01T06:51:24Z

@kerneltime Still active I think, could you help to review the PR?
I will resolve the conflictions later.

kerneltime · 2022-11-04T08:01:27Z

thanks @symious will get this reviewed

kerneltime · 2022-11-04T08:01:52Z

cc @duongkame @aswinshakil @tanvipenumudy

...p-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMNotLeaderException.java

...ntegration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAWithFailover.java

…etween Server and Client

DaveTeng0 · 2022-12-15T01:41:40Z

Changes look good to me!
(hope this could be merged, I definitely could utilize them in one of my current task! thanks!)

kerneltime · 2022-12-19T08:09:08Z

...p-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMNotLeaderException.java

+    this.leaderAddress = suggestedLeaderAddress;
  }

  public OMNotLeaderException(String message) {


We should not support this. The caller has to specify either the peer ID or the leader ID.

kerneltime · 2022-12-19T08:09:56Z

The overall change looks good and would help debug as well. Some minor nits that need addressing.

swamirishi · 2023-01-29T19:47:08Z

@kerneltime The review comments have been addressed. I guess this can be merged.

kerneltime · 2023-01-30T05:12:33Z

@swamirishi thanks for the update. I will take a look.
@neils-dev can you please revisit the PR as well?

...op-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/OMFailoverProxyProviderBase.java

...op-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/GrpcOMFailoverProxyProvider.java

symious · 2023-02-07T07:23:05Z

@neils-dev Updated the PR, please have a look.

...ne/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java

neils-dev · 2023-02-14T01:50:55Z

Thanks @symious . I tested failover on the docker ha dev cluster and noticed that when leader node goes down, the followers respond in one of two ways, either i.) initially providing a stale om leader or ii,) null address om suggestion.

i.) (stale leader suggestion) ozone-ha-om2-1 | 2023-02-14 01:22:51 DEBUG OzoneManagerProtocolServerSideTranslatorPB:251 - OM:om2 is not the leader. Suggested leader is OM:om3[om3/172.27.0.9].

ii.) ozone-ha-om2-1 | 2023-02-14 01:24:07 DEBUG OzoneManagerProtocolServerSideTranslatorPB:251 - OM:om2 is not the leader. Suggested leader is OM:om3[null].

The null address in the suggestion seems to be effective causing the failover provider to choose the next om to try from its om node map and resolves the failover.

Q. should the node give a stale om leader id, is it possible that we encounter a loop condition where we continue to try the suggestion, fail, then ask the same om for the suggestion that we already tried?
https://github.com/symious/ozone/blob/98caa74e8870829f974e77b89da6d293b7db62cd/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/OMFailoverProxyProviderBase.java#L189

symious · 2023-02-14T03:06:15Z

@neils-dev Thank you for the review. Updated the patch, could you have a check?

I think the problem you mentioned happens when the leader is not generated, once the leader is confirmed, the client should be forwarded to the correct OM.

neils-dev

Thanks @symious for the updates. LGTM.
@kerneltime do you mind taking a look? If no further comments we should look to merge.

neils-dev · 2023-02-22T03:38:47Z

Thanks @JacksonYao287 , @kerneltime , @DaveTeng0 , @swamirishi for your review and comments. Thanks @symious for this. Merge to master.

JacksonYao287 requested a review from hanishakoneru May 13, 2022 10:14

HDDS-6743. Specify leader node for OM failover

14235e2

symious force-pushed the HDDS-6743 branch from 9a219bb to 14235e2 Compare November 1, 2022 08:38

symious added 2 commits November 1, 2022 18:15

HDDS-6743. Fix findbugs

c6a70ab

trigger new CI check

40203af

kerneltime requested review from ChenSammi, kerneltime and neils-dev and removed request for hanishakoneru November 4, 2022 08:02

neils-dev reviewed Nov 7, 2022

View reviewed changes

...p-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMNotLeaderException.java Outdated Show resolved Hide resolved

...ntegration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAWithFailover.java Show resolved Hide resolved

symious added 3 commits November 7, 2022 16:50

HDDS-6743. Failover to prior implementation if OM Nodes don't match b…

92be1cc

…etween Server and Client

HDDS-6743. Add port for S3 Grpc

772b97f

trigger new CI check

801fb3e

kerneltime reviewed Dec 19, 2022

View reviewed changes

HDDS-6743. Remove one constructor

3f8bee6

neils-dev reviewed Feb 7, 2023

View reviewed changes

...op-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/OMFailoverProxyProviderBase.java Outdated Show resolved Hide resolved

neils-dev reviewed Feb 7, 2023

View reviewed changes

...op-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/GrpcOMFailoverProxyProvider.java Outdated Show resolved Hide resolved

HDDS-6743. Fix as comments

98caa74

neils-dev reviewed Feb 11, 2023

View reviewed changes

...ne/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java Outdated Show resolved Hide resolved

neils-dev reviewed Feb 13, 2023

View reviewed changes

...ne/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java Outdated Show resolved Hide resolved

neils-dev reviewed Feb 13, 2023

View reviewed changes

...ne/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java Outdated Show resolved Hide resolved

symious and others added 2 commits February 14, 2023 10:12

Merge branch 'master' into HDDS-6743

600bc8d

HDDS-6743. reuse functions

e00d9ce

neils-dev approved these changes Feb 14, 2023

View reviewed changes

Merge remote-tracking branch 'origin/master' into HDDS-6743

2200d04

neils-dev merged commit c9bbb03 into apache:master Feb 22, 2023

HDDS-6743. Specify leader node for OM failover #3409

HDDS-6743. Specify leader node for OM failover #3409

Uh oh!

Conversation

symious commented May 13, 2022

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

symious commented May 13, 2022

Uh oh!

JacksonYao287 commented May 13, 2022

Uh oh!

symious commented May 13, 2022

Uh oh!

kerneltime commented Oct 31, 2022

Uh oh!

DaveTeng0 commented Nov 1, 2022

Uh oh!

symious commented Nov 1, 2022

Uh oh!

kerneltime commented Nov 4, 2022

Uh oh!

kerneltime commented Nov 4, 2022

Uh oh!

Uh oh!

Uh oh!

DaveTeng0 commented Dec 15, 2022

Uh oh!

kerneltime Dec 19, 2022

Choose a reason for hiding this comment

Uh oh!

symious Dec 19, 2022

Choose a reason for hiding this comment

Uh oh!

kerneltime commented Dec 19, 2022

Uh oh!

swamirishi commented Jan 29, 2023

Uh oh!

kerneltime commented Jan 30, 2023

Uh oh!

Uh oh!

Uh oh!

symious commented Feb 7, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

neils-dev commented Feb 14, 2023

Uh oh!

symious commented Feb 14, 2023

Uh oh!

neils-dev left a comment

Choose a reason for hiding this comment

Uh oh!

neils-dev commented Feb 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants