You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tamas Mate (Code Review)" <ge...@cloudera.org> on 2019/06/18 17:25:35 UTC
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory...
Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13670
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory...
......................................................................
IMPALA-7734: Catalog and Statestore memz page shows useless memory...
The 'MemTracker' is removed from StatestoreD main and CatalogD main as
it is not used. When MemTracker is not available printing those memory
metrics are skipped.
Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
---
M be/src/catalog/catalogd-main.cc
M be/src/statestore/statestored-main.cc
M be/src/util/default-path-handlers.cc
M www/memz.tmpl
4 files changed, 26 insertions(+), 26 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/13670/3
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 9:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/3801/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 12:20:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
IMPALA-7734: Catalog and Statestore memz page shows useless memory
The MemTracker is removed from StatestoreD main and CatalogD main as
it is not used. The /memz page related metrics are refactored to a
a separate file. When the MemTracker is not available during callback
initialization it should not be part of the callback either. Therefore,
when MemTracker is not part of the callback it is not displayed.
Before/after pictures of the /memz page can be found in the Jira.
Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
---
M be/src/catalog/catalogd-main.cc
M be/src/runtime/exec-env.cc
M be/src/statestore/statestored-main.cc
M be/src/util/CMakeLists.txt
M be/src/util/default-path-handlers.cc
M be/src/util/default-path-handlers.h
A be/src/util/memusage-path-handlers.cc
A be/src/util/memusage-path-handlers.h
M www/memz.tmpl
9 files changed, 193 insertions(+), 90 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/13670/10
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 6:
(3 comments)
Overall, re-factoring LGTM. Just some docs comments.
http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.h
File be/src/util/memusage-path-handlers.h:
http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.h@33
PS6, Line 33: /// Adds a set of memory usage metrics to the webserver to display
Should add more docs, like what memory usage metrics are added and when.
http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.cc
File be/src/util/memusage-path-handlers.cc:
http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.cc@32
PS6, Line 32: void AddMemTracker(MemTracker* mem_tracker,
would be nice to add some docs to each of these methods
http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.cc@67
PS6, Line 67:
nit: extra new line?
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 26 Jun 2019 17:59:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4507/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jun 2019 06:56:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13670/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13670/3//COMMIT_MSG@7
PS3, Line 7:
> remove ...
Done
http://gerrit.cloudera.org:8080/#/c/13670/3/be/src/util/default-path-handlers.cc
File be/src/util/default-path-handlers.cc:
http://gerrit.cloudera.org:8080/#/c/13670/3/be/src/util/default-path-handlers.cc@135
PS3, Line 135: if (mem_tracker != nullptr
> It's less nested to do if (mem_tracker == nullptr) return;
The DCHECK was removed because it was only guarding against null MemTracker. The decision whether to print the MemTracker metrics or not was added instead.
I might be misunderstanding something.
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 19:50:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory...
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory...
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3657/ : 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/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 18:07:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3661/ : 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/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 20:32:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
IMPALA-7734: Catalog and Statestore memz page shows useless memory
The MemTracker is removed from StatestoreD main and CatalogD main as
it is not used. The /memz page related metrics are refactored to a
a separate file. When the MemTracker is not available during callback
initialization it should not be part of the callback either. Therefore,
when MemTracker is not part of the callback it is not displayed.
Before/after pictures of the /memz page can be found in the Jira.
Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
---
M be/src/catalog/catalogd-main.cc
M be/src/statestore/statestored-main.cc
M be/src/util/CMakeLists.txt
M be/src/util/default-path-handlers.cc
M be/src/util/default-path-handlers.h
A be/src/util/memusage-path-handlers.cc
A be/src/util/memusage-path-handlers.h
M www/memz.tmpl
8 files changed, 194 insertions(+), 85 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/13670/9
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory...
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory...
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13670/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13670/3//COMMIT_MSG@7
PS3, Line 7: ...
remove ...
http://gerrit.cloudera.org:8080/#/c/13670/3/be/src/util/default-path-handlers.cc
File be/src/util/default-path-handlers.cc:
http://gerrit.cloudera.org:8080/#/c/13670/3/be/src/util/default-path-handlers.cc@135
PS3, Line 135: if (mem_tracker != NULL) {
It's less nested to do if (mem_tracker == nullptr) return;
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 18:56:00 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 13:23:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
IMPALA-7734: Catalog and Statestore memz page shows useless memory
The MemTracker is removed from StatestoreD main and CatalogD main as
it is not used. When MemTracker is not available the new method that
responsible for adding mem trackers will be skipped and will not be
part of the callback. This results missing document members which
are not printed.
Before/after pictures of the '/memz' page can be found in the Jira.
Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
---
M be/src/catalog/catalogd-main.cc
M be/src/statestore/statestored-main.cc
M be/src/util/default-path-handlers.cc
M www/memz.tmpl
4 files changed, 30 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/13670/5
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 11:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4574/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 13:23:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 11: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 11
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 19:14:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 13:22:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3679/ : 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/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Jun 2019 15:49:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3802/ : 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/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 10
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 13:20:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Jun 2019 12:02:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
IMPALA-7734: Catalog and Statestore memz page shows useless memory
The MemTracker is removed from StatestoreD main and CatalogD main as
it is not used. The /memz page related metrics are refactored to a
a separate file. When the MemTracker is not available during callback
initialization it should not be part of the callback either. Therefore,
when MemTracker is not part of the callback it is not displayed.
Before/after pictures of the /memz page can be found in the Jira.
Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
---
M be/src/catalog/catalogd-main.cc
M be/src/statestore/statestored-main.cc
M be/src/util/CMakeLists.txt
M be/src/util/default-path-handlers.cc
A be/src/util/memusage-path-handlers.cc
A be/src/util/memusage-path-handlers.h
M www/memz.tmpl
7 files changed, 178 insertions(+), 85 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/13670/6
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
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/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
IMPALA-7734: Catalog and Statestore memz page shows useless memory
The MemTracker is removed from StatestoreD main and CatalogD main as
it is not used. The /memz page related metrics are refactored to a
a separate file. When the MemTracker is not available during callback
initialization it should not be part of the callback either. Therefore,
when MemTracker is not part of the callback it is not displayed.
Before/after pictures of the /memz page can be found in the Jira.
Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Reviewed-on: http://gerrit.cloudera.org:8080/13670
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/catalog/catalogd-main.cc
M be/src/runtime/exec-env.cc
M be/src/statestore/statestored-main.cc
M be/src/util/CMakeLists.txt
M be/src/util/default-path-handlers.cc
M be/src/util/default-path-handlers.h
A be/src/util/memusage-path-handlers.cc
A be/src/util/memusage-path-handlers.h
M www/memz.tmpl
9 files changed, 193 insertions(+), 90 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 12
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3718/ : 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/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Sun, 23 Jun 2019 13:04:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 4:
(1 comment)
For web UI changes, I generally find it useful to attach screenshots to the JIRA of what exactly changed. It helps document things more clearly.
http://gerrit.cloudera.org:8080/#/c/13670/4/be/src/util/default-path-handlers.cc
File be/src/util/default-path-handlers.cc:
http://gerrit.cloudera.org:8080/#/c/13670/4/be/src/util/default-path-handlers.cc@a134
PS4, Line 134:
A nice thing about this DCHECK is that it prevented callers from inadvertently passing in a nullptr for mem_tracker, but now that we have removed the DCHECK, a client might accidentally pass in a nullptr when they didn't intend to.
I would suggest re-factoring the code so there are multiple MemUsageHandler methods, one that takes in a mem_tracker, and one that does not.
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 21:15:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Hello Fredy Wijaya, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13670
to look at the new patch set (#4).
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
IMPALA-7734: Catalog and Statestore memz page shows useless memory
The 'MemTracker' is removed from StatestoreD main and CatalogD main as
it is not used. When MemTracker is not available printing those memory
metrics are skipped.
Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
---
M be/src/catalog/catalogd-main.cc
M be/src/statestore/statestored-main.cc
M be/src/util/default-path-handlers.cc
M www/memz.tmpl
4 files changed, 26 insertions(+), 26 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/13670/4
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-7734: Catalog and Statestore memz page shows useless memory
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/13670 )
Change subject: IMPALA-7734: Catalog and Statestore memz page shows useless memory
......................................................................
Patch Set 6:
(3 comments)
Thank you for the review Sahil.
Please let me know if you would add additional docs/descriptions.
http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.h
File be/src/util/memusage-path-handlers.h:
http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.h@33
PS6, Line 33: /// Adds a set of memory usage metrics to the webserver to display
> Should add more docs, like what memory usage metrics are added and when.
Added extra docs, the memory metric descriptions were
moved to the Add* helper functions.
http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.cc
File be/src/util/memusage-path-handlers.cc:
http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.cc@32
PS6, Line 32: void AddMemTracker(MemTracker* mem_tracker,
> would be nice to add some docs to each of these methods
Done
http://gerrit.cloudera.org:8080/#/c/13670/6/be/src/util/memusage-path-handlers.cc@67
PS6, Line 67:
> nit: extra new line?
Done
--
To view, visit http://gerrit.cloudera.org:8080/13670
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a42434b98aec1d9e0be6ae52325049378fdde23
Gerrit-Change-Number: 13670
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Jul 2019 09:05:19 +0000
Gerrit-HasComments: Yes