You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2017/08/03 04:52:02 UTC
[Impala-ASF-CR] IMPALA-5756: start memory maintenance thread after metric creation
Tim Armstrong has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7573
Change subject: IMPALA-5756: start memory maintenance thread after metric creation
......................................................................
IMPALA-5756: start memory maintenance thread after metric creation
Daemons were sometimes crashing on startup if the memory maintenance
thread started running before the memory metrics were created.
This changes the startup sequence so that the thread is started
after the metrics are registered.
Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
---
M be/src/catalog/catalogd-main.cc
M be/src/common/init.cc
M be/src/common/init.h
M be/src/service/impalad-main.cc
M be/src/statestore/statestored-main.cc
5 files changed, 12 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/7573/1
--
To view, visit http://gerrit.cloudera.org:8080/7573
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-5756: start memory maintenance thread after metric creation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: IMPALA-5756: start memory maintenance thread after metric creation
......................................................................
IMPALA-5756: start memory maintenance thread after metric creation
Daemons were sometimes crashing on startup if the memory maintenance
thread started running before the memory metrics were created.
This changes the startup sequence so that the thread is started
after the metrics are registered.
Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
Reviewed-on: http://gerrit.cloudera.org:8080/7573
Reviewed-by: Henry Robinson <he...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/catalog/catalogd-main.cc
M be/src/common/init.cc
M be/src/common/init.h
M be/src/service/impalad-main.cc
M be/src/statestore/statestored-main.cc
5 files changed, 13 insertions(+), 8 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Henry Robinson: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/7573
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-5756: start memory maintenance thread after metric creation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5756: start memory maintenance thread after metric creation
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/974/
--
To view, visit http://gerrit.cloudera.org:8080/7573
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5756: start memory maintenance thread after metric creation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5756: start memory maintenance thread after metric creation
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/7573
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5756: start memory maintenance thread after metric creation
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-5756: start memory maintenance thread after metric creation
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/7573/1/be/src/common/init.cc
File be/src/common/init.cc:
Line 250: memory_maintenance_thread.reset(
What happened to the is_test() logic?
Line 251: new Thread("common", "memory-maintenance-thread", &MemoryMaintenanceThread));
is there some DCHECK we can put on line 250? (something like AggregateMetrics::NUM_SMAPS != nullptr?)
http://gerrit.cloudera.org:8080/#/c/7573/1/be/src/service/impalad-main.cc
File be/src/service/impalad-main.cc:
Line 93: Status status = exec_env.StartServices();
Isn't this where RegisterMemoryMetrics() is called?
--
To view, visit http://gerrit.cloudera.org:8080/7573
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5756: start memory maintenance thread after metric creation
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-5756: start memory maintenance thread after metric creation
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/7573/1/be/src/common/init.cc
File be/src/common/init.cc:
Line 250: memory_maintenance_thread.reset(
> What happened to the is_test() logic?
We don't call this function from backend or frontend tests now so it isn't needed.
Line 251: new Thread("common", "memory-maintenance-thread", &MemoryMaintenanceThread));
> is there some DCHECK we can put on line 250? (something like AggregateMetri
Good idea - that immediately repro'd the bug.
http://gerrit.cloudera.org:8080/#/c/7573/1/be/src/service/impalad-main.cc
File be/src/service/impalad-main.cc:
Line 93: Status status = exec_env.StartServices();
> Isn't this where RegisterMemoryMetrics() is called?
Yes, you're absolutely right.
--
To view, visit http://gerrit.cloudera.org:8080/7573
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5756: start memory maintenance thread after metric creation
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-5756: start memory maintenance thread after metric creation
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7573
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5756: start memory maintenance thread after metric creation
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7573
to look at the new patch set (#2).
Change subject: IMPALA-5756: start memory maintenance thread after metric creation
......................................................................
IMPALA-5756: start memory maintenance thread after metric creation
Daemons were sometimes crashing on startup if the memory maintenance
thread started running before the memory metrics were created.
This changes the startup sequence so that the thread is started
after the metrics are registered.
Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
---
M be/src/catalog/catalogd-main.cc
M be/src/common/init.cc
M be/src/common/init.h
M be/src/service/impalad-main.cc
M be/src/statestore/statestored-main.cc
5 files changed, 13 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/7573/2
--
To view, visit http://gerrit.cloudera.org:8080/7573
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-5756: start memory maintenance thread after metric creation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5756: start memory maintenance thread after metric creation
......................................................................
Patch Set 1: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/974/
--
To view, visit http://gerrit.cloudera.org:8080/7573
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5756: start memory maintenance thread after metric creation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5756: start memory maintenance thread after metric creation
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/977/
--
To view, visit http://gerrit.cloudera.org:8080/7573
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5756: start memory maintenance thread after metric creation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5756: start memory maintenance thread after metric creation
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/976/
--
To view, visit http://gerrit.cloudera.org:8080/7573
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No