You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org> on 2023/08/16 19:58:40 UTC

[Impala-ASF-CR] WIP IMPALA-12156: Support High Availability for Statestore

Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20372


Change subject: WIP IMPALA-12156: Support High Availability for Statestore
......................................................................

WIP IMPALA-12156: Support High Availability for Statestore

This patch adds support for Statestore High Availability
- Define new service for Statestore HA.
- Negotiate role for HA with peer statestore instance on startup.
- Create HA monitor thread:
  active statestored send heartbeat to standby statestored.
  standby statestored monitor peer's connection states
- Standby statestored send heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored save the connection state as failure detecer.
- When standby statestored loss connection with active statestore,
  check the connection states for active statestore, and take over
  active role if majority of subscribers lose connections with active
  statestore.
- New active statestored send RPC notification to all subscribers
  for new active statestored and active catalogd elected by new
  active staetstored.
- New active statestored start to send heartbeat to its peer when it
  receive handshake from its peer.
- Subscriber register to two statestoreds.
- Subscriber report connection state for in-active statestore.
- Subscriber switch to new active statstore, refuse topic update
  from standby statestored.

Testings:
  TODO

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M tests/common/impala_cluster.py
M tests/common/impala_service.py
20 files changed, 1,178 insertions(+), 85 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/1
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13878/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 30 Aug 2023 00:16:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13905/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 31 Aug 2023 22:33:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 16:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14210/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 16
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 20 Oct 2023 01:46:58 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@115
PS14, Line 115: // Flags for Statestore HA
> It looks like we're somewhat inconsistent. Some use state_store, others use
Ugh, found 2 examples of statestored. Just not consistent at all.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 15
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 19 Oct 2023 17:41:51 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#18). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecter.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active statestored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statestore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests for statestored HA.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,538 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/18
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20372

to look at the new patch set (#3).

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha".

- Define new service for Statestore HA.
- Negotiate role for HA with peer statestore instance on startup.
- Create HA monitor thread:
  active statestored send heartbeat to standby statestored.
  standby statestored monitor peer's connection states
- Standby statestored send heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored save the connection state as failure detecer.
- When standby statestored loss connection with active statestore,
  check the connection states for active statestore, and take over
  active role if majority of subscribers lose connections with active
  statestore.
- New active statestored send RPC notification to all subscribers
  for new active statestored and active catalogd elected by new
  active staetstored.
- New active statestored start to send heartbeat to its peer when it
  receive handshake from its peer.
- Subscriber register to two statestoreds.
- Subscriber report connection state for in-active statestore.
- Subscriber switch to new active statstore, refuse topic update
  from standby statestored.

Testings:
 - Added end-to-end unit tests.
 - TODO: pass core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
22 files changed, 2,086 insertions(+), 106 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/3
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 13:

(19 comments)

http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore-subscriber.cc
File be/src/statestore/statestore-subscriber.cc:

http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore-subscriber.cc@178
PS11, Line 178:           "statestored (protocol version $0), the request is ignored.",
> added log messages
Done


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore-subscriber.cc@326
PS11, Line 326:     return statestore_->Start();
> If FLAGS_tolerate_statestore_startup_delay is enabled, subscriber enter rec
Done


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore-subscriber.cc@358
PS11, Line 358:     IpAddr ip_address;
> statestore_id is received in registration response. Due to race, subscriber
Ack


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore-subscriber.cc@410
PS11, Line 410:   } else {
> done
Done


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.h
File be/src/statestore/statestore.h:

http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.h@137
PS11, Line 137: /// the subscribers. Active statestored also sends heartbeats to standby statestored.
> done
Done


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.h@140
PS11, Line 140: /// heartbeats from active statestored.
> Will write warning message on standby statestored in this case.
Ack


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.h@782
PS11, Line 782: 
> Updated comment to the purpose of this variable.
Ack


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@654
PS11, Line 654:   DCHECK(metrics != NULL);
> Thanks to catch this. Least significant 64 bits of statestore_id is not use
Oh, I actually missed the UUIDToTUniqueId above. But it still wasn't clear what setting lower bits was used for.


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1248
PS11, Line 1248:       } else if (active_conn_states_[subscriber->id()] == TStatestoreConnState::FAILED) {
> added DCHECK
Done


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1250
PS11, Line 1250:             || conn_state == TStatestoreConnState::UNKNOWN);
> added DCHECK
Done


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1251
PS11, Line 1251:         --failed_conn_state_count_;
> added DCHECK
Done


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1573
PS11, Line 1573:   // to all subscribers.
> timeout is used for re-sending HA updates if there are subscribers which fa
But it holds a unique_lock on ha_lock_. Won't that prevent MonitorStatestoredHaHeartbeat from doing anything while WaitFor is waiting?


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1654
PS11, Line 1654:           request.__set_catalogd_version(active_catalogd_version);
> updated as suggested
Done


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1792
PS11, Line 1792:   // Negotiate role for HA with peer statestore instance on startup.
> Both designate themselves as active, and start to send heartbeats to each o
Done


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1963
PS11, Line 1963:       // Check if standby statestored is reachable.
> Don't have good plan to handle split-brain in network. Updated comments. An
You added a warning below. I don't have any better ideas at the moment.


http://gerrit.cloudera.org:8080/#/c/20372/11/common/thrift/StatestoreService.thrift
File common/thrift/StatestoreService.thrift:

http://gerrit.cloudera.org:8080/#/c/20372/11/common/thrift/StatestoreService.thrift@499
PS11, Line 499:   // statestore should set itself as inactive if dst_statestore_active is set as true.
> done
Done


http://gerrit.cloudera.org:8080/#/c/20372/11/common/thrift/StatestoreService.thrift@500
PS11, Line 500:   // Otherwise, source statestore sets itself as active.
> done
Done


http://gerrit.cloudera.org:8080/#/c/20372/11/tests/custom_cluster/test_statestored_ha.py
File tests/custom_cluster/test_statestored_ha.py:

http://gerrit.cloudera.org:8080/#/c/20372/11/tests/custom_cluster/test_statestored_ha.py@42
PS11, Line 42: 
> done
Done


http://gerrit.cloudera.org:8080/#/c/20372/11/tests/custom_cluster/test_statestored_ha.py@48
PS11, Line 48:     request = Subscriber.TSetStatestoreDebugActionRequest(
> removed
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 13
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 27 Sep 2023 18:59:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 13:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/20372/11//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20372/11//COMMIT_MSG@31
PS11, Line 31:   Standby statestored saves the connection state as failure detecter.
> done
Done


http://gerrit.cloudera.org:8080/#/c/20372/11//COMMIT_MSG@38
PS11, Line 38:   active statestored.
> done
Done


http://gerrit.cloudera.org:8080/#/c/20372/11//COMMIT_MSG@48
PS11, Line 48: - Subscribers switch to new active statestore when receiving RPC
> done
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 13
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 27 Sep 2023 18:59:37 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 14:

(15 comments)

A few small things to look at but overall looks great.

Also, please do not forget to update the ports list doc -- https://impala.apache.org/docs/build/asf-site-html/topics/impala_ports.html

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@435
PS14, Line 435: DEFINE_string(peer_state_store_host, "localhost",
Can this flag be moved to be/src/statestore/statestore.cc?  Since it is statestore specific, it would make more sense there.  That being said, Impala has some inconsistencies with flag placement/naming thus this is not a must do for me.

Also, renaming it to 'state_store_peer_host' would better match existing naming conventions


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@437
PS14, Line 437: DEFINE_int32(peer_state_store_ha_port, 24021,
Can this flag be moved to be/src/statestore/statestore.cc?  Since it is statestore specific, it would make more sense there.  That being said, Impala has some inconsistencies with flag placement/naming thus this is not a must do for me.

Also, renaming it to 'state_store_peer_ha_port' would better match existing naming conventions


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@439
PS14, Line 439: DEFINE_bool(enable_statestored_ha, false, "Set to true to enable Statestore HA");
To better match conventions, rename this to 'enable_state_store_ha'


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@440
PS14, Line 440: DEFINE_bool(force_statestored_active, false, "Set to true to force this statestored "
Can this flag be moved to be/src/statestore/statestore.cc?  Since it is statestore specific, it would make more sense there.  That being said, Impala has some inconsistencies with flag placement/naming thus this is not a must do for me.

Also, renaming it to 'state_store_force_active' would better match existing naming conventions


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@446
PS14, Line 446: DEFINE_bool(use_network_address_as_statestored_priority, false, "Network address is "
Rename to 'use_network_address_as_state_store_priority' to better match conventions.

Also, is this flag applicable only to statestored?  If so, can it be moved to Can this flag be moved to be/src/statestore/statestore.cc?


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@452
PS14, Line 452: DEFINE_int64(statestored_ha_preemption_wait_period_ms, 10000, "(Advanced) The time after "
Can this flag be moved to be/src/statestore/statestore.cc?  Since it is statestore specific, it would make more sense there.  That being said, Impala has some inconsistencies with flag placement/naming thus this is not a must do for me.

Also, renaming it to 'state_store_ha_preemption_wait_period_ms' would better match existing naming conventions


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc
File be/src/statestore/statestore-subscriber.cc:

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@125
PS14, Line 125:       LOG(INFO) << Substitute("Receive UpdateState RPC request from incompatible "
To me, this should be a warning instead of info.  If this situation does happen, it probably means something is wrong in the cluster setup.  An info level message will get lost too easily.


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@158
PS14, Line 158:       LOG(INFO) << Substitute("Receive Heartbeat RPC request from incompatible "
To me, this should be a warning instead of info.  If this situation does happen, it probably means something is wrong in the cluster setup.  An info level message will get lost too easily.


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@177
PS14, Line 177:       LOG(INFO) << Substitute("Receive UpdateCatalogd RPC request from incompatible "
To me, this should be a warning instead of info.  If this situation does happen, it probably means something is wrong in the cluster setup.  An info level message will get lost too easily.


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@205
PS14, Line 205:       LOG(INFO) << Substitute("Receive UpdateStatestoredRole RPC request from "
To me, this should be a warning instead of info.  If this situation does happen, it probably means something is wrong in the cluster setup.  An info level message will get lost too easily.


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@461
PS14, Line 461:   // DCHECK(active_statestore_ != nullptr);
Nit: please either remove or uncomment this code.


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@115
PS14, Line 115: // Flags for Statestore HA
All these flags should start with 'state_store' to better match existing conventions for flag names.


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@1785
PS14, Line 1785: satestored
nit: misspelled 'satestored' in thread name.


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@1983
PS14, Line 1983:       // TODO (wzhou): Need better approach to handle split-brain in network.
Please reference an IMPALA Jira here.


http://gerrit.cloudera.org:8080/#/c/20372/14/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/14/bin/start-impala-cluster.py@336
PS14, Line 336:         DEFAULT_STATESTORE_HA_SERVICE_PORT + (instance_num + 1) % 2
Nit: additional parentheses would make this code more readable even though they are not necessary due to order of operations rules:
DEFAULT_STATESTORE_HA_SERVICE_PORT + ((instance_num + 1) % 2)



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 14
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 18 Oct 2023 18:56:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#15). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecter.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active statestored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statestore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests for statestored HA.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,533 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/15
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 15
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 16:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/common/global-flags.cc@426
PS15, Line 426: DEFINE_int32(state_store_2_port, 24001,
> Thoughts on moving these flags to the same location where their correspondi
I prefer to put statestore server related flags in statestore.cc.


http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/common/global-flags.cc@432
PS15, Line 432: // TGeospatialLibrary's values are mapped here as constants
> Given the existing flags are named '-state_store_host' and '-state_store_po
Agree


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@115
PS14, Line 115: // Flags for Statestore HA
> Good point, I did some more investigating, and flag names definitely favor 
dropped the 'd' from statestore flag variables.


http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/statestore/statestore.cc@122
PS15, Line 122: DEFINE_bool(statestore_force_active, false, "Set to true to force this statestored "
> Please rename to 'statestore_force_active'
renamed as 'statestore_force_active'


http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/statestore/statestore.cc@134
PS15, Line 134: DEFINE_int64(statestore_ha_preemption_wait_period_ms, 10000, "(Advanced) The time after "
> Please rename to 'statestore_ha_preemption_wait_period_ms'
renamed as 'statestore_force_active'


http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/statestore/statestore.cc@946
PS15, Line 946: G(INFO) << "Registering: " << subscriber_id;
              :   bool is_reregistering = false;
              :   {
removed from patch #14. Brought back on patch #15 by mistake



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 16
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 20 Oct 2023 01:17:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 11:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore-subscriber.cc
File be/src/statestore/statestore-subscriber.cc:

http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore-subscriber.cc@178
PS11, Line 178:     if (status.ok()) {
We should probably log if status is not ok, right? Although it looks like we haven't done that previously anywhere.


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore-subscriber.cc@326
PS11, Line 326:       // Both statestoreds are not ready.
How does this get resolved?


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore-subscriber.cc@358
PS11, Line 358:   if (statestore_->IsMatchingStatestoreId(statestore_id) || statestore2_ == nullptr) {
Why do we need to worry about statestore2_ == nullptr here?


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore-subscriber.cc@410
PS11, Line 410:     // statstoreds.
nit: statestoreds


http://gerrit.cloudera.org:8080/#/c/20372/11/common/thrift/StatestoreService.thrift
File common/thrift/StatestoreService.thrift:

http://gerrit.cloudera.org:8080/#/c/20372/11/common/thrift/StatestoreService.thrift@499
PS11, Line 499:   // statstore should set itself as inactive if dst_statestore_active is set as true.
nit: statestore


http://gerrit.cloudera.org:8080/#/c/20372/11/common/thrift/StatestoreService.thrift@500
PS11, Line 500:   // Otherwise, source statstore sets itself as active.
nit: statestore


http://gerrit.cloudera.org:8080/#/c/20372/11/tests/custom_cluster/test_statestored_ha.py
File tests/custom_cluster/test_statestored_ha.py:

http://gerrit.cloudera.org:8080/#/c/20372/11/tests/custom_cluster/test_statestored_ha.py@42
PS11, Line 42:   as true.  """
nit: extra spaces


http://gerrit.cloudera.org:8080/#/c/20372/11/tests/custom_cluster/test_statestored_ha.py@48
PS11, Line 48:   _, port = handle.getsockname()
I don't see this used anywhere.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 11
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 22 Sep 2023 22:00:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#16). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecter.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active statestored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statestore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests for statestored HA.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,537 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/16
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 16
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 15:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14205/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 15
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 19 Oct 2023 06:54:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 16: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 16
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Mon, 23 Oct 2023 17:00:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 16:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9829/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 16
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Mon, 23 Oct 2023 20:44:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 9:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13932/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 9
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Sep 2023 16:26:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13917/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 8
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 05 Sep 2023 18:53:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 15:

(15 comments)

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@435
PS14, Line 435: // TGeospatialLibrary's values are mapped here as constants
> Can this flag be moved to be/src/statestore/statestore.cc?  Since it is sta
moved to statestore.cc and renamed to state_store_peer_host


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@437
PS14, Line 437: static const string geo_lib_hive_esri = "HIVE_ESRI";
> Can this flag be moved to be/src/statestore/statestore.cc?  Since it is sta
moved to statestore.cc and renamed as 'state_store_peer_ha_port'


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@439
PS14, Line 439: static const string geo_lib_help_msg =
> To better match conventions, rename this to 'enable_state_store_ha'
moved to statestore.cc


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@440
PS14, Line 440:     "Specifies which implementation of "
> Can this flag be moved to be/src/statestore/statestore.cc?  Since it is sta
moved to statestore.cc and renamed as statestored_force_active


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@446
PS14, Line 446: // ++========================++
> Rename to 'use_network_address_as_state_store_priority' to better match con
moved to statestore.cc


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@452
PS14, Line 452: //          /   R I P   ╲ -----------|-----------
> Can this flag be moved to be/src/statestore/statestore.cc?  Since it is sta
moved to statestore.cc


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc
File be/src/statestore/statestore-subscriber.cc:

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@125
PS14, Line 125:       LOG(WARNING) << Substitute("Receive UpdateState RPC request from incompatible "
> To me, this should be a warning instead of info.  If this situation does ha
changed to warning


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@158
PS14, Line 158:       LOG(WARNING) << Substitute("Receive Heartbeat RPC request from incompatible "
> To me, this should be a warning instead of info.  If this situation does ha
changed to warning


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@177
PS14, Line 177:       LOG(WARNING) << Substitute("Receive UpdateCatalogd RPC request from incompatible "
> To me, this should be a warning instead of info.  If this situation does ha
changed to warning


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@205
PS14, Line 205:       LOG(WARNING) << Substitute("Receive UpdateStatestoredRole RPC request from "
> To me, this should be a warning instead of info.  If this situation does ha
WARNING


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@461
PS14, Line 461: StatestoreSubscriber::StatestoreStub* StatestoreSubscriber::GetActiveStatestore() {
> Nit: please either remove or uncomment this code.
removed


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@115
PS14, Line 115: // Flags for Statestore HA
> All these flags should start with 'state_store' to better match existing co
All existing flags in this source file are started with 'statestore'.


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@1785
PS14, Line 1785: 
> nit: misspelled 'satestored' in thread name.
fixed


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@1983
PS14, Line 1983:           ha_standby_ss_failure_detector_->GetPeerState(STATESTORE_ID);
> Please reference an IMPALA Jira here.
Filed a jira IMPALA-12507


http://gerrit.cloudera.org:8080/#/c/20372/14/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/14/bin/start-impala-cluster.py@336
PS14, Line 336:         DEFAULT_STATESTORE_HA_SERVICE_PORT + ((instance_num + 1) % 2)
> Nit: additional parentheses would make this code more readable even though 
changed as suggested



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 15
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 19 Oct 2023 06:26:48 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@395
PS17, Line 395:     statestored_arg_list.append(args)
Oh, is this building a list of lists? It should use extend.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 16:38:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13891/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 30 Aug 2023 23:10:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 11:

(14 comments)

I've made it about halfway, posting some questions.

http://gerrit.cloudera.org:8080/#/c/20372/11//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20372/11//COMMIT_MSG@31
PS11, Line 31:   Standby statestored saves the connection state as failure detecer.
nit: detector


http://gerrit.cloudera.org:8080/#/c/20372/11//COMMIT_MSG@38
PS11, Line 38:   active staetstored.
nit: statestored


http://gerrit.cloudera.org:8080/#/c/20372/11//COMMIT_MSG@48
PS11, Line 48: - Subscribers switch to new active statstore when receiving RPC
nit: statestore


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.h
File be/src/statestore/statestore.h:

http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.h@137
PS11, Line 137: /// the subscribers. Active statetsored also sends heartbeats to standby statestored.
nit: statestored


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.h@140
PS11, Line 140: /// heartbeats from active statestored.
We could have a failure scenario where active statestored is still sending heartbeats to standby, but majority of subscribers are unable to reach the active statestored (and only standby). That seems like an acceptable exception where manual intervention would be needed.


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.h@782
PS11, Line 782:   int64_t active_version_ = 0;
Why is this based on time rather than an incrementing counter? It looks like it's just used for causal ordering.

It would help to describe the purpose of this variable.


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@654
PS11, Line 654:   // Set least significant 64 bits of statestore_id as the number of microseconds that
Doc string for FLAGS_use_network_address_as_statestored_priority says if false it'll use a random value, but this isn't really random.

In most cases whichever statestore was started last will have priority. It's not clear to me whether that will cause the newer statestore to claim primary. Does the negotiation only happen if the older instance is a standby (i.e. both startup in a similar time window, and not yet timeout on the initial handshake)?


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1248
PS11, Line 1248:         ++failed_conn_state_count_;
Can we add a DCHECK for what we expect the old state to be?


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1250
PS11, Line 1250:         --failed_conn_state_count_;
Can we add a DCHECK for what we expect the new state to be?


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1251
PS11, Line 1251:       }
What are the other options here? Let's enforce them via DCHECK.


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1573
PS11, Line 1573:       update_statestored_cv_.WaitFor(l, timeout_us);
This combination means that we'll only be able to process other HA updates when the timeout occurs, right?


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1654
PS11, Line 1654:           if (status.code() == TErrorCode::RPC_RECV_TIMEOUT) {
nit: This and conditional line above could be combined to reduce indentation.


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1792
PS11, Line 1792:     // statestored designates itself as active if the statestore can not connect to the
What happens if both instances do this?


http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1963
PS11, Line 1963:         // TODO wzhou How to handle split-brain in network?
It looks like you outlined a plan here but haven't yet implemented it?



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 11
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 21 Sep 2023 23:53:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/12/tests/custom_cluster/test_statestored_ha.py
File tests/custom_cluster/test_statestored_ha.py:

http://gerrit.cloudera.org:8080/#/c/20372/12/tests/custom_cluster/test_statestored_ha.py@21
PS12, Line 21: import socket
flake8: F401 'socket' imported but unused



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 12
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 27 Sep 2023 05:14:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/4/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/4/bin/start-impala-cluster.py@794
PS4, Line 794: e
> flake8: E131 continuation line unaligned for hanging indent
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 30 Aug 2023 00:18:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] WIP IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: WIP IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20372/1/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/20372/1/be/src/common/global-flags.cc@410
PS1, Line 410: DEFINE_int64(update_statestored_rpc_resend_interval_ms, 100, "(Advanced) Interval (in ms) "
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/20372/1/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/20372/1/be/src/statestore/statestore.cc@1705
PS1, Line 1705:   RETURN_IF_ERROR(Thread::Create("statestore-update-satestored", "update-statestored-thread",
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/20372/1/be/src/statestore/statestore.cc@1742
PS1, Line 1742:                 DebugAction(FLAGS_debug_actions, "STATESTORE_HA_HANDSHAKE_FIRST_ATTEMPT") :
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/20372/1/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/1/bin/start-impala-cluster.py@634
PS1, Line 634:  
flake8: E241 multiple spaces after ','



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 16 Aug 2023 19:59:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#14). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecter.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active statestored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statestore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests for statestored HA.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,534 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/14
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 14
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecer.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active staetstored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statstore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,488 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/9
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 9
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 17:

(1 comment)

Containerized tests seem to be consistently failing to start.

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@379
PS17, Line 379: def build_statestored_arg_list(num_statestored, remap_ports):
I suspect something added to the return arg list for this function isn't a string, and that's causing __run_container__ to fail in containerized tests. I'd need to dig through it more to figure out where exactly that's happening.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 17
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 16:25:59 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#19). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecter.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active statestored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statestore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests for statestored HA.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,541 insertions(+), 114 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/19
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecer.
- When standby statestored losses connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers have losed connections with
  active statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active staetstored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statstore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.

Testings:
 - Added end-to-end unit tests.
 - TODO: pass core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
22 files changed, 2,146 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/6
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13858/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 28 Aug 2023 17:12:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/20372/3/be/src/statestore/statestore-subscriber.cc
File be/src/statestore/statestore-subscriber.cc:

http://gerrit.cloudera.org:8080/#/c/20372/3/be/src/statestore/statestore-subscriber.cc@865
PS3, Line 865:       // Try to acquire lock to avoid race with updating catalogd from registration
> line too long (91 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/20372/3/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/3/bin/start-impala-cluster.py@793
PS3, Line 793: 
> flake8: E501 line too long (94 > 90 characters)
Done


http://gerrit.cloudera.org:8080/#/c/20372/3/tests/custom_cluster/test_statestored_ha.py
File tests/custom_cluster/test_statestored_ha.py:

http://gerrit.cloudera.org:8080/#/c/20372/3/tests/custom_cluster/test_statestored_ha.py@24
PS3, Line 24: from tests.common.impala_cluster import DEFAULT_CATALOG_SERVICE_PORT
> flake8: F401 'tests.common.skip.SkipIf' imported but unused
Done


http://gerrit.cloudera.org:8080/#/c/20372/3/tests/custom_cluster/test_statestored_ha.py@143
PS3, Line 143: p
> flake8: F821 undefined name 'pytest'
Done


http://gerrit.cloudera.org:8080/#/c/20372/3/tests/custom_cluster/test_statestored_ha.py@320
PS3, Line 320:  
> flake8: E131 continuation line unaligned for hanging indent
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 28 Aug 2023 17:24:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha".

- Define new service for Statestore HA.
- Negotiate role for HA with peer statestore instance on startup.
- Create HA monitor thread:
  active statestored send heartbeat to standby statestored.
  standby statestored monitor peer's connection states
- Standby statestored send heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored save the connection state as failure detecer.
- When standby statestored loss connection with active statestore,
  check the connection states for active statestore, and take over
  active role if majority of subscribers lose connections with active
  statestore.
- New active statestored send RPC notification to all subscribers
  for new active statestored and active catalogd elected by new
  active staetstored.
- New active statestored start to send heartbeat to its peer when it
  receive handshake from its peer.
- Subscriber register to two statestoreds.
- Subscriber report connection state for in-active statestore.
- Subscriber switch to new active statstore, refuse topic update
  from standby statestored.

Testings:
 - Added end-to-end unit tests.
 - TODO: pass core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
22 files changed, 2,088 insertions(+), 106 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/4
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] WIP IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: WIP IMPALA-12156: Support High Availability for Statestore
......................................................................

WIP IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha".

- Define new service for Statestore HA.
- Negotiate role for HA with peer statestore instance on startup.
- Create HA monitor thread:
  active statestored send heartbeat to standby statestored.
  standby statestored monitor peer's connection states
- Standby statestored send heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored save the connection state as failure detecer.
- When standby statestored loss connection with active statestore,
  check the connection states for active statestore, and take over
  active role if majority of subscribers lose connections with active
  statestore.
- New active statestored send RPC notification to all subscribers
  for new active statestored and active catalogd elected by new
  active staetstored.
- New active statestored start to send heartbeat to its peer when it
  receive handshake from its peer.
- Subscriber register to two statestoreds.
- Subscriber report connection state for in-active statestore.
- Subscriber switch to new active statstore, refuse topic update
  from standby statestored.

Testings:
  TODO

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
21 files changed, 1,487 insertions(+), 87 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/2
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 14: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/20372/11/be/src/statestore/statestore.cc@1573
PS11, Line 1573:   while (1) {
> ConditionVariable::WaitFor() calls pthread_cond_timedwait(). pthread_cond_t
Ah thanks, I forgot how that worked.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 14
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 27 Sep 2023 23:41:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 16: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9829/


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 16
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 01:18:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 18:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14235/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 17:03:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 19:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9838/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 18:11:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 19:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9837/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 17:13:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecter.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active statestored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statestore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests for statestored HA.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Reviewed-on: http://gerrit.cloudera.org:8080/20372
Reviewed-by: Michael Smith <mi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,541 insertions(+), 114 deletions(-)

Approvals:
  Michael Smith: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 20
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecer.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active staetstored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statstore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests for statestored HA.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,495 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/11
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 11
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13859/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 28 Aug 2023 17:50:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecer.
- When standby statestored losses connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers have losed connections with
  active statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active staetstored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statstore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.

Testings:
 - Added end-to-end unit tests.
 - TODO: pass core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
22 files changed, 2,138 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/5
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecer.
- When standby statestored losses connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers have losed connections with
  active statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active staetstored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statstore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.

Testings:
 - TODO: simulate network failures with tools like 'tc' to make
   statestored temporarily lose network connections with other
   nodes in the cluster.
 - Added end-to-end unit tests.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
22 files changed, 2,180 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/7
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 10:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13935/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Sep 2023 19:08:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@379
PS17, Line 379: def build_statestored_arg_list(num_statestored, remap_ports):
> I suspect something added to the return arg list for this function isn't a 
Found the cause. This function returns a list When calling this function from DockerMiniClusterOperations.start_statestore() with enable_statestored_ha not enabled, it should use the first element.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 16:33:01 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@379
PS17, Line 379: def build_statestored_arg_list(num_statestored, remap_ports):
> Found the cause. This function returns a list When calling this function fr
I'd like to see the function description updated reflecting that the return value format has changed.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 16:40:30 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 16: Code-Review+1

(4 comments)

looks good to me!

http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/common/global-flags.cc@426
PS15, Line 426: DEFINE_int32(state_store_2_port, 24001,
> Thoughts on moving these flags to the same location where their correspondi
Done


http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/common/global-flags.cc@432
PS15, Line 432: // TGeospatialLibrary's values are mapped here as constants
> Given the existing flags are named '-state_store_host' and '-state_store_po
Done


http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/statestore/statestore.cc@122
PS15, Line 122: DEFINE_bool(statestore_force_active, false, "Set to true to force this statestored "
> renamed as 'statestore_force_active'
Done


http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/statestore/statestore.cc@134
PS15, Line 134: DEFINE_int64(statestore_ha_preemption_wait_period_ms, 10000, "(Advanced) The time after "
> Please rename to 'statestore_ha_preemption_wait_period_ms'
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 16
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 20 Oct 2023 22:10:49 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 19:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14236/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 17:42:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecer.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active staetstored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statstore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests for statestored HA.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,495 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/10
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 13:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14087/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 13
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 27 Sep 2023 06:12:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 14:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14093/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 14
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 28 Sep 2023 00:00:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/4/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/4/bin/start-impala-cluster.py@794
PS4, Line 794: c
flake8: E131 continuation line unaligned for hanging indent



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 28 Aug 2023 17:25:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] WIP IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: WIP IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/20372/2/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/2/bin/start-impala-cluster.py@355
PS2, Line 355: def statestored_service_name(i):
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/20372/2/bin/start-impala-cluster.py@791
PS2, Line 791: D
flake8: F602 dictionary key variable DEFAULT_STATESTORE_HA_SERVICE_PORT repeated with different values


http://gerrit.cloudera.org:8080/#/c/20372/2/bin/start-impala-cluster.py@792
PS2, Line 792: D
flake8: F602 dictionary key variable DEFAULT_STATESTORE_HA_SERVICE_PORT repeated with different values



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Aug 2023 04:45:41 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 15:

(20 comments)

A few more thoughts/questions.

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@435
PS14, Line 435: // TGeospatialLibrary's values are mapped here as constants
> moved to statestore.cc and renamed to state_store_peer_host
Done


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@437
PS14, Line 437: static const string geo_lib_hive_esri = "HIVE_ESRI";
> moved to statestore.cc and renamed as 'state_store_peer_ha_port'
Done


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@439
PS14, Line 439: static const string geo_lib_help_msg =
> To better match conventions, rename this to 'enable_state_store_ha'
Done


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@440
PS14, Line 440:     "Specifies which implementation of "
> moved to statestore.cc and renamed as statestored_force_active
Done


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@446
PS14, Line 446: // ++========================++
> Rename to 'use_network_address_as_state_store_priority' to better match con
Done


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/common/global-flags.cc@452
PS14, Line 452: //          /   R I P   ╲ -----------|-----------
> moved to statestore.cc
Done


http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/common/global-flags.cc@426
PS15, Line 426: // Starting flags for Statestore HA
Thoughts on moving these flags to the same location where their corresponding original flags are?

For example, 'state_store_host' is defined in exec-env.cc while 'state_store_port' is defined in statestore.cc.  Does that mean 'state_store_2_host' should also go into exec-env.cc and 'state_store_2_port' go into statestore.cc?

I don't like have the flags spread out across so many files, but I also prefer to stick with existing code layout.


http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/common/global-flags.cc@432
PS15, Line 432: DEFINE_int32(state_store_ha_port, 24020,
Given the existing flags are named '-state_store_host' and '-state_store_port', I think we should stick with these flags also having a name that starts with '-state_store_' instead of '-statestore_'


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc
File be/src/statestore/statestore-subscriber.cc:

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@125
PS14, Line 125:       LOG(WARNING) << Substitute("Receive UpdateState RPC request from incompatible "
> changed to warning
Done


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@158
PS14, Line 158:       LOG(WARNING) << Substitute("Receive Heartbeat RPC request from incompatible "
> changed to warning
Done


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@177
PS14, Line 177:       LOG(WARNING) << Substitute("Receive UpdateCatalogd RPC request from incompatible "
> changed to warning
Done


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@205
PS14, Line 205:       LOG(WARNING) << Substitute("Receive UpdateStatestoredRole RPC request from "
> WARNING
Done


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore-subscriber.cc@461
PS14, Line 461: StatestoreSubscriber::StatestoreStub* StatestoreSubscriber::GetActiveStatestore() {
> removed
Done


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@115
PS14, Line 115: // Flags for Statestore HA
> All existing flags in this source file are started with 'statestore'.
Done


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@115
PS14, Line 115: // Flags for Statestore HA
> It looks like we're somewhat inconsistent. Some use state_store, others use
Good point, I did some more investigating, and flag names definitely favor starting with '-statestore_' instead of '-state_store_'.

I did not find any other instance of a flag starting with '-statestored', thus I agree we should drop the 'd'.

Overall, I agree that the new flags should start with '-statestore_' instead of my original thought of starting with '-state_store_'


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@115
PS14, Line 115: // Flags for Statestore HA
> All these flags should start with 'state_store' to better match existing co
Done


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@1785
PS14, Line 1785: 
> fixed
Done


http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@1983
PS14, Line 1983:           ha_standby_ss_failure_detector_->GetPeerState(STATESTORE_ID);
> Filed a jira IMPALA-12507
Done


http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/statestore/statestore.cc@122
PS15, Line 122: DEFINE_bool(statestored_force_active, false, "Set to true to force this statestored "
Please rename to 'statestore_force_active'


http://gerrit.cloudera.org:8080/#/c/20372/15/be/src/statestore/statestore.cc@134
PS15, Line 134: DEFINE_int64(statestored_ha_preemption_wait_period_ms, 10000, "(Advanced) The time after "
Please rename to 'statestore_ha_preemption_wait_period_ms'



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 15
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 19 Oct 2023 20:12:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@395
PS17, Line 395:     statestored_arg_list.append(args)
> Oh, is this building a list of lists? It should use extend.
Oh I see, that's intentional because it's a separate arg list for each instance. Now I understand your response at 379.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 16:39:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 19: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@786
PS17, Line 786:   def start_statestore(self):
I'm curious why this implementation looks so different from start_catalogd.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 17:17:17 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 19:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@786
PS17, Line 786:   def start_statestore(self):
> For catalogd HA, the two catalog instances don't communicate each other. Th
Ack



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 18:10:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 17:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9831/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 17
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 02:14:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 19: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 21:34:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 12:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14086/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 12
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 27 Sep 2023 05:40:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecter.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active statestored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statestore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests for statestored HA.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,537 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/13
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 13
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 11:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14018/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 11
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Mon, 18 Sep 2023 18:00:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecer.
- When standby statestored losses connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers have losed connections with
  active statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active staetstored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statstore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,451 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/8
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 8
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] WIP IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: WIP IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13798/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Aug 2023 05:09:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/20372/3/be/src/statestore/statestore-subscriber.cc
File be/src/statestore/statestore-subscriber.cc:

http://gerrit.cloudera.org:8080/#/c/20372/3/be/src/statestore/statestore-subscriber.cc@865
PS3, Line 865:       // Try to acquire lock to avoid race with updating catalogd from registration thread.
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/20372/3/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/3/bin/start-impala-cluster.py@793
PS3, Line 793: t
flake8: E501 line too long (94 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/20372/3/tests/custom_cluster/test_statestored_ha.py
File tests/custom_cluster/test_statestored_ha.py:

http://gerrit.cloudera.org:8080/#/c/20372/3/tests/custom_cluster/test_statestored_ha.py@24
PS3, Line 24: from tests.common.skip import SkipIf
flake8: F401 'tests.common.skip.SkipIf' imported but unused


http://gerrit.cloudera.org:8080/#/c/20372/3/tests/custom_cluster/test_statestored_ha.py@143
PS3, Line 143: p
flake8: F821 undefined name 'pytest'


http://gerrit.cloudera.org:8080/#/c/20372/3/tests/custom_cluster/test_statestored_ha.py@320
PS3, Line 320: "
flake8: E131 continuation line unaligned for hanging indent



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 28 Aug 2023 16:47:30 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 15: Code-Review+1

Option naming is slightly inconsistent, but that's nothing new.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 15
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 19 Oct 2023 17:42:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 17: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 17
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 02:14:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@395
PS17, Line 395:     statestored_arg_list.append(args)
> Oh, is this building a list of lists? It should use extend.
Yes, it's building a list of lists.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 16:46:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 19:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@786
PS17, Line 786:   def start_statestore(self):
> I'm curious why this implementation looks so different from start_catalogd.
For catalogd HA, the two catalog instances don't communicate each other. The active statestored decide which catalogd instance is active. For statestored HA, two statestore instance communicate to negotiate the active role, the active statestored send heartbeat to standby statestored. There are additional port settings for statestore HA.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 17:56:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 19:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/20372/17/bin/start-impala-cluster.py@379
PS17, Line 379: def build_statestored_arg_list(num_statestored, remap_ports):
> I'd like to see the function description updated reflecting that the return
Updated description.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 17:13:30 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/20372/14/be/src/statestore/statestore.cc@115
PS14, Line 115: // Flags for Statestore HA
> All existing flags in this source file are started with 'statestore'.
It looks like we're somewhat inconsistent. Some use state_store, others use statestore. I don't see any examples of statestored_ though, so maybe we should drop the 'd' from these options.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 15
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 19 Oct 2023 17:41:11 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20372/12/tests/custom_cluster/test_statestored_ha.py
File tests/custom_cluster/test_statestored_ha.py:

http://gerrit.cloudera.org:8080/#/c/20372/12/tests/custom_cluster/test_statestored_ha.py@21
PS12, Line 21: import socket
> flake8: F401 'socket' imported but unused
removed it.



-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 12
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 27 Sep 2023 05:22:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................

IMPALA-12156: Support High Availability for Statestore

To support statestore HA, we allow two statestored instances in an
Active-Passive HA pair to be added to an Impala cluster. We add the
preemptive behavior for statestored. When HA is enabled, the preemptive
behavior allows the statestored with the higher priority to become
active and the paired statestored becomes standby. The active
statestored acts as the owner of Impala cluster and provides statestore
service for the cluster members.

To enable catalog HA for a cluster, two statestoreds in the HA pair and
all subscribers must be started with starting flag
"enable_statestored_ha" as true.

This patch makes following changes:
- Defined new service for Statestore HA.
- Statestored negotiates the role for HA with its peer statestore
  instance on startup.
- Create HA monitor thread:
  Active statestored sends heartbeat to standby statestored.
  Standby statestored monitors peer's connection states with their
  subscribers.
- Standby statestored sends heartbeat to subscribers with request
  for connection state between active statestore and subscribers.
  Standby statestored saves the connection state as failure detecter.
- When standby statestored lost connection with active statestore,
  it checks the connection states for active statestore, and takes over
  active role if majority of subscribers lost connections with active
  statestore.
- New active statestored sends RPC notification to all subscribers
  for new active statestored and active catalogd elected by the new
  active statestored.
- New active statestored starts sending heartbeat to its peer when it
  receives handshake from its peer.
- Active statestored enters recovery mode if it lost connections with
  its peer statestored and all subscribers. It keeps sending HA
  handshake to its peer until receiving response.
- All subscribers (impalad/catalogd/admissiond) register to two
  statestoreds.
- Subscribers report connection state for the requests from standby
  statestore.
- Subscribers switch to new active statestore when receiving RPC
  notifications from new active statestored.
- Only active statestored sends topic update messages to subscribers.
- Add a new option "enable_statestored_ha" in script
  bin/start-impala-cluster.py for starting Impala mini-cluster with
  statestored HA enabled.
- Add a new Thrift API in statestore service to disable network
  for statestored. It's only used for unit-test to simulate network
  failure. For safety, it's only working when the debug action is set
  in starting flags.

Testings:
 - Added end-to-end unit tests for statestored HA.
 - Passed core tests

Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/common/global-flags.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/admissiond-env.cc
M be/src/statestore/statestore-service-client-wrapper.h
M be/src/statestore/statestore-subscriber-catalog.cc
M be/src/statestore/statestore-subscriber-catalog.h
M be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M bin/start-impala-cluster.py
M common/thrift/StatestoreService.thrift
M common/thrift/metrics.json
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_statestored_ha.py
23 files changed, 2,538 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/20372/12
-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 12
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] WIP IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: WIP IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13765/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 16 Aug 2023 20:24:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12156: Support High Availability for Statestore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20372 )

Change subject: IMPALA-12156: Support High Availability for Statestore
......................................................................


Patch Set 17: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9831/


-- 
To view, visit http://gerrit.cloudera.org:8080/20372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd2c814bbad5c04c1d50c2edaa5b910c82a6fd87
Gerrit-Change-Number: 20372
Gerrit-PatchSet: 17
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ar...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Oct 2023 06:38:57 +0000
Gerrit-HasComments: No