You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Xiang Yang (Code Review)" <ge...@cloudera.org> on 2022/09/27 12:12:23 UTC

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

Xiang Yang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19029


Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................

IMPALA-11606: add 'untracked memory' metric.

Add a gauge metric 'untracked memroy' to record the memory size that
not tracked by the 'process' mem-tracker.

Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
---
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/util/memory-metrics.cc
M be/src/util/memory-metrics.h
M common/thrift/metrics.json
5 files changed, 47 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 2
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 3: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 3
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 29 Sep 2022 18:08:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric

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

Change subject: IMPALA-11606: add 'untracked memory' metric
......................................................................


Patch Set 8:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 20 Oct 2022 10:03:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19029/7/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/19029/7/be/src/runtime/mem-tracker.cc@56
PS7, Line 56:   MemTracker* mem_tracker_;
How about `const MemTracker* mem_tracker_` if `mem_tracker_` is not changed after UntrackedMemoryMetric created.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 7
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Wed, 12 Oct 2022 09:01:11 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric

Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Hello Xianqing He, Jian Zhang, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-11606: add 'untracked memory' metric
......................................................................

IMPALA-11606: add 'untracked memory' metric

Add a gauge metric 'untracked memroy' to record the memory size that
not tracked by the 'process' mem-tracker.

Testing:
 - Manually request the debug UI metrics interface and compare the
   value with the untracked memory in memz page.
 - Run core jobs.

Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
---
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M common/thrift/metrics.json
3 files changed, 46 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Hello Xianqing He, Jian Zhang, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................

IMPALA-11606: add 'untracked memory' metric.

Add a gauge metric 'untracked memroy' to record the memory size that
not tracked by the 'process' mem-tracker.

Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
---
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M common/thrift/metrics.json
3 files changed, 41 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 6
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric

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

Change subject: IMPALA-11606: add 'untracked memory' metric
......................................................................


Patch Set 8:

(8 comments)

Thanks. I have some suggestions, but I'm not very familiar with this area so it would be good if someone else could also have a look.

http://gerrit.cloudera.org:8080/#/c/19029/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19029/8//COMMIT_MSG@9
PS8, Line 9: that
           : not
that is not


http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.h
File be/src/runtime/mem-tracker.h:

http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.h@305
PS8, Line 305: UntrackedMemory
The name is a bit misleading because what it returns is memory tracked by this MemTracker but not its children.
The comment should explain this.
'MemoryUntrackedByChildren()' may be a better name.

I know it's intended for process level MemTrackers where "not tracked by children" more or less means "untracked" but this function can be called on other kinds of MemTrackers, too, where the name is misleading.


http://gerrit.cloudera.org:8080/#/c/19029/7/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/19029/7/be/src/runtime/mem-tracker.cc@56
PS7, Line 56:   MemTracker* mem_tracker_;
> Hi jian, unfortunately I can't  add the const modifier because it cannot co
It should be
  MemTracker* const mem_tracker_;
That is, the pointer cannot be changed but the pointed-to value can.


http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc@49
PS8, Line 49: IntGauge
No need to inherit from IntGauge (which is a typedef for AtomicMetric<TMetricKind::GAUGE>, see https://github.com/apache/impala/blob/1899b2e34b3fd184bcd3101fb5da8ae80479ef25/be/src/util/metrics-fwd.h#L66).

AtomicMetric adds the member "AtomicInt64 value_;" to ScalarMetric, which we don't use here, as well as the member functions "SetValue()" and "Increment()" which are not appropriate here.

We should inherit from ScalarMetric<int64_t, TMetricKind::GAUGE> instead.


http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc@273
PS8, Line 273: auto
Writing the type would be more informative.


http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc@274
PS8, Line 274:   metrics->RegisterMetric(bytes_untracked);
I'm a bit concerned about the lifetime of this MemTracker and  the UntrackedMemoryMetric instance, which holds a pointer to 'this'. 

I think we only call this function when this MemTracker and the MetricGroup object, which will have ownership of the UntrackedMemoryMetric object, have ~equal lifetimes (for example as members of ExecEnv), but if I'm wrong or the code changes in the future, it could lead to a dangling pointer.

I'm not sure there is a good way to check that as I don't think the order of destroying MemTracker and MetricGroup (together with the UntrackedMemoryMetric object) is fixed. Unless we enforce an order, we'd have to have both objects notify the other when they are destroyed, checking whether the other is still alive. That would be complicated.

We could at least add a comment to RegisterMetrics() in the header saying that this function should only be called for process level MemTrackers. Please correct me if we actually call it for other MemTrackers, I may be wrong.


http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc@401
PS8, Line 401:   // TODO: uncomment next line will cause the 'TestAdmissionController::()
How does the test fail?


http://gerrit.cloudera.org:8080/#/c/19029/8/common/thrift/metrics.json
File common/thrift/metrics.json:

http://gerrit.cloudera.org:8080/#/c/19029/8/common/thrift/metrics.json@2043
PS8, Line 2043: that not
Nit: that is not



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 08 Nov 2022 14:40:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 7
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Wed, 12 Oct 2022 07:33:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 3
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 29 Sep 2022 23:21:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric

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

Change subject: IMPALA-11606: add 'untracked memory' metric
......................................................................


Patch Set 8:

Hi Daniel, can you help me review this? thanks.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Sat, 29 Oct 2022 05:10:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 4
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Fri, 30 Sep 2022 08:49:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric

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

Change subject: IMPALA-11606: add 'untracked memory' metric
......................................................................


Patch Set 8: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Mon, 24 Oct 2022 06:02:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 5: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 5
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Sat, 08 Oct 2022 08:04:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 6:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 6
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Wed, 12 Oct 2022 07:43:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19029/2/be/src/util/memory-metrics.h
File be/src/util/memory-metrics.h:

http://gerrit.cloudera.org:8080/#/c/19029/2/be/src/util/memory-metrics.h@37
PS2, Line 37: class ExecEnv;
nit: suggest ordering alphabetically



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 2
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 29 Sep 2022 03:50:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 5
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Sat, 08 Oct 2022 03:00:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19029/5/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/19029/5/be/src/runtime/mem-tracker.cc@56
PS5, Line 56:   shared_ptr<MemTracker> mem_tracker_;
I think you don’t need a smart pointer here. 'mem_tracker' does not need maintenance in this class



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 5
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Mon, 10 Oct 2022 11:52:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 2
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Sep 2022 12:33:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 3
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 29 Sep 2022 13:05:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric

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

Change subject: IMPALA-11606: add 'untracked memory' metric
......................................................................


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc@272
PS8, Line 272: bytes_untracked
nit: "bytes-untracked" fits more with the other metric names.


http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc@396
PS8, Line 396:     lock_guard<SpinLock> l(child_trackers_lock_);
Do you have chance to test this in a busy cluster? Not sure if crawling the metrics every seconds or 0.5s will impact normal workload due to acquiring this lock.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Wed, 09 Nov 2022 00:16:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric

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

Change subject: IMPALA-11606: add 'untracked memory' metric
......................................................................


Patch Set 8:

Hi joe, can you help me review this? thanks.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 01 Nov 2022 09:04:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: Add the 'mem-tracker.process.bytes-untracked' metric.

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

Change subject: IMPALA-11606: Add the 'mem-tracker.process.bytes-untracked' metric.
......................................................................


Patch Set 9:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/19029/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19029/8//COMMIT_MSG@9
PS8, Line 9: rd
           : the
> that is not
Done


http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.h
File be/src/runtime/mem-tracker.h:

http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.h@305
PS8, Line 305: ld only apply t
> The name is a bit misleading because what it returns is memory tracked by t
Done


http://gerrit.cloudera.org:8080/#/c/19029/7/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/19029/7/be/src/runtime/mem-tracker.cc@56
PS7, Line 56:   MemTracker* const mem_tra
> It should be
Done


http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc@49
PS8, Line 49: ScalarMe
> No need to inherit from IntGauge (which is a typedef for AtomicMetric<TMetr
Done


http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc@272
PS8, Line 272: 
> nit: "bytes-untracked" fits more with the other metric names.
Done


http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc@273
PS8, Line 273: TMet
> Writing the type would be more informative.
Done


http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc@274
PS8, Line 274:   UntrackedMemoryMetric* bytes_untracked = new UntrackedMemoryMetric(this, metric_def);
> I'm a bit concerned about the lifetime of this MemTracker and  the Untracke
Done


http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc@396
PS8, Line 396:   {
> Do you have chance to test this in a busy cluster? Not sure if crawling the
I ran an internal benchmark (which contains 79 internal sqls, executed serially) on a 3 nodes cluster  for 3 times, with a script to crawl the metrics every seconds  or not, the total executing times are almost the same(whether crawling the metrics or not).

Or should I make it a flag to control whether register the metric or not?


http://gerrit.cloudera.org:8080/#/c/19029/8/be/src/runtime/mem-tracker.cc@401
PS8, Line 401:   }
> How does the test fail?
I can't figure it out :(


http://gerrit.cloudera.org:8080/#/c/19029/8/common/thrift/metrics.json
File common/thrift/metrics.json:

http://gerrit.cloudera.org:8080/#/c/19029/8/common/thrift/metrics.json@2043
PS8, Line 2043: that is 
> Nit: that is not
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 9
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Wed, 15 Feb 2023 02:30:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11606: Add the 'mem-tracker.process.bytes-untracked' metric.

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

Change subject: IMPALA-11606: Add the 'mem-tracker.process.bytes-untracked' metric.
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19029/9/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/19029/9/be/src/runtime/mem-tracker.cc@402
PS9, Line 402: TODO
> Let's not forget that this should be resolved before merging this change :)
I found that if I sleep 10s in TestAdmissionController::test_timeout_reason_host_memory() without this patch, it failed as well, this is because the MemoryMaintenanceThread sleep 10s before refresh the process mem-tracker: https://github.com/apache/impala/blob/4.2.0/be/src/common/init.cc#L202, and after the refresh action, the consuption increased from 0 to ~175MB(I have to admit that I haven't figure out the memory calculate logic yet), but the expected 2MB. 
It looks like this testcase isn't implemented correctly.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 9
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Sun, 14 May 2023 15:52:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric

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

Change subject: IMPALA-11606: add 'untracked memory' metric
......................................................................


Patch Set 8:

(2 comments)

> Patch Set 5:
> 
> (1 comment)

http://gerrit.cloudera.org:8080/#/c/19029/5/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/19029/5/be/src/runtime/mem-tracker.cc@56
PS5, Line 56:   MemTracker* mem_tracker_;
> I think you don’t need a smart pointer here. 'mem_tracker' does not need ma
Done


http://gerrit.cloudera.org:8080/#/c/19029/7/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/19029/7/be/src/runtime/mem-tracker.cc@56
PS7, Line 56:   MemTracker* mem_tracker_;
> How about `const MemTracker* mem_tracker_` if `mem_tracker_` is not changed
Hi jian, unfortunately I can't  add the const modifier because it cannot compatible with the lock_guard in UntrackedMemory().



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 8
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 20 Oct 2022 10:12:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 3
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Fri, 30 Sep 2022 04:23:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 3: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 3
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Fri, 30 Sep 2022 04:03:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 2
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Comment-Date: Wed, 28 Sep 2022 03:24:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 4: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 4
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Fri, 30 Sep 2022 13:58:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: Add the 'mem-tracker.process.bytes-untracked' metric.

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

Change subject: IMPALA-11606: Add the 'mem-tracker.process.bytes-untracked' metric.
......................................................................


Patch Set 9:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/19029/9//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19029/9//COMMIT_MSG@10
PS9, Line 10: it's
            : children
Should be something like "but not by its children".


http://gerrit.cloudera.org:8080/#/c/19029/9//COMMIT_MSG@11
PS9, Line 11: , it
Could be a full stop (.) and start a new sentence.


http://gerrit.cloudera.org:8080/#/c/19029/9//COMMIT_MSG@11
PS9, Line 11:  when continue increasing
Nit: when it continues to increase.


http://gerrit.cloudera.org:8080/#/c/19029/9/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/19029/9/be/src/runtime/mem-tracker.cc@260
PS9, Line 260: WARN
Nit: "WARNING" would be better.


http://gerrit.cloudera.org:8080/#/c/19029/9/be/src/runtime/mem-tracker.cc@402
PS9, Line 402: TODO
Let's not forget that this should be resolved before merging this change :)


http://gerrit.cloudera.org:8080/#/c/19029/9/common/thrift/metrics.json
File common/thrift/metrics.json:

http://gerrit.cloudera.org:8080/#/c/19029/9/common/thrift/metrics.json@2043
PS9, Line 2043: but it's children
but not by its children



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 9
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 23 Feb 2023 12:38:30 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11606: Add the 'mem-tracker.process.bytes-untracked' metric.

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

Change subject: IMPALA-11606: Add the 'mem-tracker.process.bytes-untracked' metric.
......................................................................


Patch Set 9:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 9
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Wed, 15 Feb 2023 02:40:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: Add the 'mem-tracker.process.bytes-untracked' metric.

Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Xianqing He, Jian Zhang, Daniel Becker, Joe McDonnell, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-11606: Add the 'mem-tracker.process.bytes-untracked' metric.
......................................................................

IMPALA-11606: Add the 'mem-tracker.process.bytes-untracked' metric.

Add a gauge metric 'mem-tracker.process.bytes-untracked' to record
the memory size that is tracked by the 'process' mem-tracker but it's
children, it usually indicates a memory leak when continue increasing.

Testing:
 - Manually request the debug UI metrics interface and compare the
   value with the untracked memory value in memz page.
 - Run core jobs.

Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
---
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M common/thrift/metrics.json
3 files changed, 48 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 9
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 3
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Thu, 29 Sep 2022 13:19:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Hello Xianqing He, Jian Zhang, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................

IMPALA-11606: add 'untracked memory' metric.

Add a gauge metric 'untracked memroy' to record the memory size that
not tracked by the 'process' mem-tracker.

Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
---
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M common/thrift/metrics.json
3 files changed, 41 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 3
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Hello Xianqing He, Jian Zhang, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................

IMPALA-11606: add 'untracked memory' metric.

Add a gauge metric 'untracked memroy' to record the memory size that
not tracked by the 'process' mem-tracker.

Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
---
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M common/thrift/metrics.json
3 files changed, 41 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 7
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 7:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 7
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Wed, 12 Oct 2022 07:54:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11606: add 'untracked memory' metric.

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

Change subject: IMPALA-11606: add 'untracked memory' metric.
......................................................................


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib16e00109d732f759c96c7a967eb1cc32124a03f
Gerrit-Change-Number: 19029
Gerrit-PatchSet: 7
Gerrit-Owner: Xiang Yang <yx...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Wed, 12 Oct 2022 12:36:17 +0000
Gerrit-HasComments: No