You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org> on 2020/02/28 22:31:30 UTC

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

Thomas Tauber-Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15321


Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................

IMPALA-9425 (part 1): Introduce uuids for impalads

This patch introduces the concept of 'backend ids', which are unique
ids that can be used to identify individual impalads. The ids are
generated by each impalad on startup.

The patch then uses the ids to fix a bug where the statestore may fail
to inform coordinators when an executor has failed and restarted. The
bug was caused by the fact that the statestore cluster membership
topic was keyed on statestore subscriber ids, which are host:port
pairs.

So, if an impalad fails and a new one is started at the same host:port
before a particular coordinator has a cluster membership update
generated for it by the statestore, the statestore has no way of
differentiating the prior impalad from the newly started impalad, and
the topic update will not show the deletion of the original impalad.

With this patch, the cluster membership topic is now keyed by backend
id, so since the restarted impalad will have a different backend id
the next membership update after the prior impalad failed is
guaranteed to reflect that failure.

Further patches will apply the backend ids in other places where we
currently key off host:port pairs to identify impalads.

Testing:
- Added an e2e test that uses a new debug action to add delay to
  statestore topic updates. Due to the use of JITTER the test is
  non-deterministic, but it repros the original issue locally for me
  about 50% of the time.
- Passed a full run of existing tests.

Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
---
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/statestore/statestore.cc
M common/thrift/StatestoreService.thrift
M common/thrift/Types.thrift
M tests/common/impala_test_suite.py
M tests/custom_cluster/test_restart_services.py
12 files changed, 95 insertions(+), 50 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/15321 )

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 4: Code-Review+2

(2 comments)

carrying forward

http://gerrit.cloudera.org:8080/#/c/15321/3/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/15321/3/be/src/service/impala-server.h@467
PS3, Line 467: ids not i
> nit: ids?
Done


http://gerrit.cloudera.org:8080/#/c/15321/3/be/src/service/impala-server.h@1287
PS3, Line 1287: running
> nit: running
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Mar 2020 18:49:27 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15321 )

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 3: Code-Review+1

(4 comments)

Hi Thomas, thanks for addressing my comments. I do not have any other comment.

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/scheduling/cluster-membership-mgr.cc@135
PS2, Line 135:   bool ss_is_recovering = statestore_subscriber_ != nullptr
> Well this is used in the next line (line 140)
Thanks! Sorry that I missed that 'ss_is_recovering' is also used at line 140.


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/scheduling/cluster-membership-mgr.cc@347
PS2, Line 347: }
> Yes, that's correct
Thanks for the confirmation!


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc@506
PS2, Line 506:           static_cast<uint64_t>(location.second.query_ids.size()),
> Addresses in another comment. Short version is that I think we probably ult
Thanks for the explanation!


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-server.cc@1723
PS2, Line 1723:   }
> The motivation for using 'address' here instead of 'backend_id' was that I 
Thanks for the detailed explanation!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Mar 2020 16:57:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 3: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15321/3/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/15321/3/be/src/service/impala-server.h@467
PS3, Line 467: addresses
nit: ids?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Mar 2020 18:13:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5411/ : 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/15321
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Mar 2020 23:06:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Fang-Yu Rao, Sahil Takiar, Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................

IMPALA-9425 (part 1): Introduce uuids for impalads

This patch introduces the concept of 'backend ids', which are unique
ids that can be used to identify individual impalads. The ids are
generated by each impalad on startup.

The patch then uses the ids to fix a bug where the statestore may fail
to inform coordinators when an executor has failed and restarted. The
bug was caused by the fact that the statestore cluster membership
topic was keyed on statestore subscriber ids, which are host:port
pairs.

So, if an impalad fails and a new one is started at the same host:port
before a particular coordinator has a cluster membership update
generated for it by the statestore, the statestore has no way of
differentiating the prior impalad from the newly started impalad, and
the topic update will not show the deletion of the original impalad.

With this patch, the cluster membership topic is now keyed by backend
id, so since the restarted impalad will have a different backend id
the next membership update after the prior impalad failed is
guaranteed to reflect that failure.

The patch also logs the backend ids on startup and adds them to the
/backends webui page and to the query locations section of the
/queries page, for use in debugging.

Further patches will apply the backend ids in other places where we
currently key off host:port pairs to identify impalads.

Testing:
- Added an e2e test that uses a new debug action to add delay to
  statestore topic updates. Due to the use of JITTER the test is
  non-deterministic, but it repros the original issue locally for me
  about 50% of the time.
- Passed a full run of existing tests.

Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
---
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/statestore/statestore.cc
M common/thrift/StatestoreService.thrift
M common/thrift/Types.thrift
M tests/common/impala_test_suite.py
M tests/custom_cluster/test_restart_services.py
M www/backends.tmpl
M www/queries.tmpl
12 files changed, 122 insertions(+), 45 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15321 )

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 2:

(4 comments)

Hi Thomas, I have read this patch but do not have any major comment. I raised those questions mostly because I am not familiar with this part of our BE codebase and would like to check if my understanding is correct. Thank you very much!

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/scheduling/cluster-membership-mgr.cc@135
PS2, Line 135:   bool ss_is_recovering = statestore_subscriber_ != nullptr
It is not directly related to this patch. But is it ok for us to move this following statement to the place where we have known there is at least one condition in the following if-statement at Line 148 that evaluates to false? I just like to know whether or not my understanding is correct. Thanks!


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/scheduling/cluster-membership-mgr.cc@347
PS2, Line 347:   // Send notifications to all callbacks registered to receive updates.
Is it true that the callbacks referred to here correspond to the input argument to the function cluster_membership_mgr_->RegisterUpdateCallbackFn() at Line 548 in exec-env.cc? I'd just like to check if my understanding is correct.


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc@506
PS2, Line 506:         Value location_name(TNetworkAddressToString(be_entry.second.address).c_str(),
'be_entry.second' is of the type TBackendDescriptor, which has 3 fields that are of the type TNetworkAddress, i.e., 'address', 'debug_http_address', and 'krpc_address'. Is it okay to use any of the other two instead, i.e., 'debug_http_address' or 'krpc_address'? Thanks!


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-server.cc@1723
PS2, Line 1723:   map<TUniqueId, vector<TNetworkAddress>> queries_to_cancel;
Is it possible to use 'backend_id' instead of 'address' to construct the map? Or the change would be too huge since the elements in 'queries_to_cancel' will also be used in other function later on, e.g., cancellation_thread_pool_->Offer(CancellationWork::BackendFailure(cancellation_entry.first, cancellation_entry.second));"?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 02 Mar 2020 18:52:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15321 )

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................

IMPALA-9425 (part 1): Introduce uuids for impalads

This patch introduces the concept of 'backend ids', which are unique
ids that can be used to identify individual impalads. The ids are
generated by each impalad on startup.

The patch then uses the ids to fix a bug where the statestore may fail
to inform coordinators when an executor has failed and restarted. The
bug was caused by the fact that the statestore cluster membership
topic was keyed on statestore subscriber ids, which are host:port
pairs.

So, if an impalad fails and a new one is started at the same host:port
before a particular coordinator has a cluster membership update
generated for it by the statestore, the statestore has no way of
differentiating the prior impalad from the newly started impalad, and
the topic update will not show the deletion of the original impalad.

With this patch, the cluster membership topic is now keyed by backend
id, so since the restarted impalad will have a different backend id
the next membership update after the prior impalad failed is
guaranteed to reflect that failure.

The patch also logs the backend ids on startup and adds them to the
/backends webui page and to the query locations section of the
/queries page, for use in debugging.

Further patches will apply the backend ids in other places where we
currently key off host:port pairs to identify impalads.

Testing:
- Added an e2e test that uses a new debug action to add delay to
  statestore topic updates. Due to the use of JITTER the test is
  non-deterministic, but it repros the original issue locally for me
  about 50% of the time.
- Passed a full run of existing tests.

Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Reviewed-on: http://gerrit.cloudera.org:8080/15321
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/statestore/statestore.cc
M common/thrift/StatestoreService.thrift
M common/thrift/Types.thrift
M tests/common/impala_test_suite.py
M tests/custom_cluster/test_restart_services.py
M www/backends.tmpl
M www/queries.tmpl
12 files changed, 122 insertions(+), 45 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/15321 )

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 3:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/15321/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15321/2//COMMIT_MSG@32
PS2, Line 32: /queries page, for use in debugging.
> It might be worth exposing the backend ID in the debug UI, if it's also exp
Done


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/scheduling/cluster-membership-mgr.cc@135
PS2, Line 135:   bool ss_is_recovering = statestore_subscriber_ != nullptr
> It is not directly related to this patch. But is it ok for us to move this 
Well this is used in the next line (line 140)

You're right that we generally try to declare variables as close to where they're used as possible.


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/scheduling/cluster-membership-mgr.cc@347
PS2, Line 347: }
> Is it true that the callbacks referred to here correspond to the input argu
Yes, that's correct


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc@498
PS2, Line 498: 
> nit: DCHECK this is not null?
Done


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc@502
PS2, Line 502:       location_json.AddMember("location", location_name, document->GetAllocator());
> Should this be an auto& to avoid a copy?
Done


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc@503
PS2, Line 503: GetAllocat
> Tim already mentioned it, but might be good to expose the UUID here; it loo
Done


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc@506
PS2, Line 506:           static_cast<uint64_t>(location.second.query_ids.size()),
> 'be_entry.second' is of the type TBackendDescriptor, which has 3 fields tha
Addresses in another comment. Short version is that I think we probably ultimately want to add a TBackendDescriptor.hostname


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-server.h@466
PS2, Line 466:   /// Takes a set of backend ids of active backends and cancels all the queries running on
> I do wonder if the previous API is a little more robust since the cancellat
That's a really good point. I think "called for every membership topic update" is probably not a good enough guarantee, because you could for example have a situation where the statestore fails and then while we're in recovery an impalad fails and we might never get this callback executed with that impalad in 'removed_backends'

I'll go back to the way it was previously.


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-server.cc@1723
PS2, Line 1723:   }
> Is it possible to use 'backend_id' instead of 'address' to construct the ma
The motivation for using 'address' here instead of 'backend_id' was that I want 'address' available later for cancellation_thread_pool_ to be able to construct an error message that contains the hostnames rather than one that contains backend ids, since hostnames are more user-friendly. I'll add a comment explaining this.

We also can't just switch directly to using krpc address as its an ip address, not a hostname. We'll have to think about what we want to do in these sorts of situations when we remove TBackendDescriptor.address, but I think probably the right solution is to add a TBackendDescriptor.hostname and then put both that hostname and the backend_id in logging and error messages.

That's outside the scope of this patch, though.


http://gerrit.cloudera.org:8080/#/c/15321/2/common/thrift/StatestoreService.thrift
File common/thrift/StatestoreService.thrift:

http://gerrit.cloudera.org:8080/#/c/15321/2/common/thrift/StatestoreService.thrift@62
PS2, Line 62:   // Unique identifier for this impalad. Generated on startup.
> nit: add docs
Done


http://gerrit.cloudera.org:8080/#/c/15321/2/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/15321/2/tests/common/impala_test_suite.py@1080
PS2, Line 1080:   def wait_for_state(self, handle, expected_state, timeout, client=None):
              :     """Waits for the given 'query_handle' to reach the 'expected_state' using 'client', or
              :     with the default connection if 'client' is None. If it does not reach the given state
              :     within 'timeout' seconds, the method throws an AssertionError.
              :     """
              :     self.wait_for_any_state(handle, [expected_state], timeout, client)
              : 
              :   def wait_for_any_state(self, handle, expected_states, timeout, client=None):
              :     """Waits for the given 'query_handle' to reach one of 'expected_states' using 'client'
              :     or with the default connection if 'clien
> update docs for 'client' param
Done


http://gerrit.cloudera.org:8080/#/c/15321/2/tests/custom_cluster/test_restart_services.py
File tests/custom_cluster/test_restart_services.py:

http://gerrit.cloudera.org:8080/#/c/15321/2/tests/custom_cluster/test_restart_services.py@103
PS2, Line 103:       restarted at the same location."""
             :       if self.exploration_strategy() != 'exhaustive':
             :         pytest.skip()
             : 
             :       assert len(self.cluster.impalads) == 3
             :       client = self.cluster.impalads[0].serv
> nit: use CustomClusterTestSuite.with_args?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Mar 2020 22:21:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 2:

(3 comments)

The change makes a lot of sense, this is a much cleaner solution than the host/port thing.

http://gerrit.cloudera.org:8080/#/c/15321/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15321/2//COMMIT_MSG@32
PS2, Line 32: 
It might be worth exposing the backend ID in the debug UI, if it's also exposed in the logs.


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc@502
PS2, Line 502:     for (auto be_entry : membership_snapshot->current_backends) {
Should this be an auto& to avoid a copy?


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-server.h@466
PS2, Line 466:   /// Takes BackendIdMap containing backends that were removed from the cluster membership
I do wonder if the previous API is a little more robust since the cancellation would be level-triggered rather than edge triggered.

It looks like the callback is guaranteed to be called for every membership change so it doesn't really make a difference in context.

So this is fine, I'll just leave the comment anyway to document my thought process.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 29 Feb 2020 01:44:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:

http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc@498
PS2, Line 498: membership_snapshot
nit: DCHECK this is not null?


http://gerrit.cloudera.org:8080/#/c/15321/2/be/src/service/impala-http-handler.cc@503
PS2, Line 503: backend_id
Tim already mentioned it, but might be good to expose the UUID here; it looks like its possible for there to be two separate entries in the table, with the same address, so adding the uuid would help differentiate duplicate addresses


http://gerrit.cloudera.org:8080/#/c/15321/2/common/thrift/StatestoreService.thrift
File common/thrift/StatestoreService.thrift:

http://gerrit.cloudera.org:8080/#/c/15321/2/common/thrift/StatestoreService.thrift@62
PS2, Line 62:   1: required Types.BackendId backend_id;
nit: add docs


http://gerrit.cloudera.org:8080/#/c/15321/2/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/15321/2/tests/common/impala_test_suite.py@1080
PS2, Line 1080:   def wait_for_state(self, handle, expected_state, timeout, client=None):
              :     """Waits for the given 'query_handle' to reach the 'expected_state'. If it does not
              :     reach the given state within 'timeout' seconds, the method throws an AssertionError.
              :     """
              :     self.wait_for_any_state(handle, [expected_state], timeout, client)
              : 
              :   def wait_for_any_state(self, handle, expected_states, timeout, client=None):
              :     """Waits for the given 'query_handle' to reach one of 'expected_states'. If it does
              :     not reach one of the given states within 'timeout' seconds, the method throws an
              :     AssertionError. Returns the final state.
update docs for 'client' param


http://gerrit.cloudera.org:8080/#/c/15321/2/tests/custom_cluster/test_restart_services.py
File tests/custom_cluster/test_restart_services.py:

http://gerrit.cloudera.org:8080/#/c/15321/2/tests/custom_cluster/test_restart_services.py@103
PS2, Line 103:       # Debug action to delay statestore updates to give the restarted impalad time to
             :       # register itself before a membership topic update is generated.
             :       self._start_impala_cluster(
             :           ["--state_store_args=--debug_actions=DO_SUBSCRIBER_UPDATE:JITTER@10000"],
             :           num_coordinators=1, cluster_size=3)
             :       assert len(self.cluster.impalads) == 3
nit: use CustomClusterTestSuite.with_args?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 02 Mar 2020 18:53:01 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5372/ : 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/15321
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 29 Feb 2020 01:47:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Fang-Yu Rao, Sahil Takiar, Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................

IMPALA-9425 (part 1): Introduce uuids for impalads

This patch introduces the concept of 'backend ids', which are unique
ids that can be used to identify individual impalads. The ids are
generated by each impalad on startup.

The patch then uses the ids to fix a bug where the statestore may fail
to inform coordinators when an executor has failed and restarted. The
bug was caused by the fact that the statestore cluster membership
topic was keyed on statestore subscriber ids, which are host:port
pairs.

So, if an impalad fails and a new one is started at the same host:port
before a particular coordinator has a cluster membership update
generated for it by the statestore, the statestore has no way of
differentiating the prior impalad from the newly started impalad, and
the topic update will not show the deletion of the original impalad.

With this patch, the cluster membership topic is now keyed by backend
id, so since the restarted impalad will have a different backend id
the next membership update after the prior impalad failed is
guaranteed to reflect that failure.

Further patches will apply the backend ids in other places where we
currently key off host:port pairs to identify impalads.

Testing:
- Added an e2e test that uses a new debug action to add delay to
  statestore topic updates. Due to the use of JITTER the test is
  non-deterministic, but it repros the original issue locally for me
  about 50% of the time.
- Passed a full run of existing tests.

Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
---
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/statestore/statestore.cc
M common/thrift/StatestoreService.thrift
M common/thrift/Types.thrift
M tests/common/impala_test_suite.py
M tests/custom_cluster/test_restart_services.py
12 files changed, 95 insertions(+), 51 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Mar 2020 18:49:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

Thanks for addressing my comments

http://gerrit.cloudera.org:8080/#/c/15321/3/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/15321/3/be/src/service/impala-server.h@1287
PS3, Line 1287: cunning
nit: running



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Mar 2020 23:48:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 1:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/5370/ : Initial code review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Feb 2020 23:17:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Mar 2020 18:49:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Mar 2020 23:43:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Fang-Yu Rao, Sahil Takiar, Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................

IMPALA-9425 (part 1): Introduce uuids for impalads

This patch introduces the concept of 'backend ids', which are unique
ids that can be used to identify individual impalads. The ids are
generated by each impalad on startup.

The patch then uses the ids to fix a bug where the statestore may fail
to inform coordinators when an executor has failed and restarted. The
bug was caused by the fact that the statestore cluster membership
topic was keyed on statestore subscriber ids, which are host:port
pairs.

So, if an impalad fails and a new one is started at the same host:port
before a particular coordinator has a cluster membership update
generated for it by the statestore, the statestore has no way of
differentiating the prior impalad from the newly started impalad, and
the topic update will not show the deletion of the original impalad.

With this patch, the cluster membership topic is now keyed by backend
id, so since the restarted impalad will have a different backend id
the next membership update after the prior impalad failed is
guaranteed to reflect that failure.

The patch also logs the backend ids on startup and adds them to the
/backends webui page and to the query locations section of the
/queries page, for use in debugging.

Further patches will apply the backend ids in other places where we
currently key off host:port pairs to identify impalads.

Testing:
- Added an e2e test that uses a new debug action to add delay to
  statestore topic updates. Due to the use of JITTER the test is
  non-deterministic, but it repros the original issue locally for me
  about 50% of the time.
- Passed a full run of existing tests.

Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
---
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/statestore/statestore.cc
M common/thrift/StatestoreService.thrift
M common/thrift/Types.thrift
M tests/common/impala_test_suite.py
M tests/custom_cluster/test_restart_services.py
M www/backends.tmpl
M www/queries.tmpl
12 files changed, 122 insertions(+), 45 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9425 (part 1): Introduce uuids for impalads

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

Change subject: IMPALA-9425 (part 1): Introduce uuids for impalads
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5421/ : 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/15321
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf8067349ed6b765f6fed830b7140f60738e9061
Gerrit-Change-Number: 15321
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Mar 2020 19:34:04 +0000
Gerrit-HasComments: No