Aleksandr Efimov has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24485 )

Change subject: IMPALA-15088: Add ReadOnlyIntGauge
......................................................................


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/24485/1/be/src/runtime/krpc-data-stream-mgr.h
File be/src/runtime/krpc-data-stream-mgr.h:

http://gerrit.cloudera.org:8080/#/c/24485/1/be/src/runtime/krpc-data-stream-mgr.h@353
PS1, Line 353:   ReadOnlyIntGauge* deserialization_queue_size_;
> For types that are subclasses of ReadOnlyIntGauge, let's leave the variable
Fixed in PS2.


http://gerrit.cloudera.org:8080/#/c/24485/1/be/src/util/malloc-util-gperftools.h
File be/src/util/malloc-util-gperftools.h:

http://gerrit.cloudera.org:8080/#/c/24485/1/be/src/util/malloc-util-gperftools.h@50
PS1, Line 50: class TcmallocMetric : public IntGauge {
> I would also make this inherit from ReadOnlyIntGauge.
Fixed in PS2.


http://gerrit.cloudera.org:8080/#/c/24485/1/be/src/util/malloc-util-libc.h
File be/src/util/malloc-util-libc.h:

http://gerrit.cloudera.org:8080/#/c/24485/1/be/src/util/malloc-util-libc.h@33
PS1, Line 33: class LibcMallocMetric : public IntGauge {
> This can inherit from ReadOnlyIntGauge as it doesn't need to support the se
Fixed in PS2.


http://gerrit.cloudera.org:8080/#/c/24485/1/be/src/util/malloc-util-sanitizers.h
File be/src/util/malloc-util-sanitizers.h:

http://gerrit.cloudera.org:8080/#/c/24485/1/be/src/util/malloc-util-sanitizers.h@33
PS1, Line 33: class SanitizerMallocMetric : public IntGauge {
> This can inherit from ReadOnlyIntGauge as it doesn't need to support the se
Fixed in PS2.


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

http://gerrit.cloudera.org:8080/#/c/24485/1/be/src/util/memory-metrics.h@53
PS1, Line 53:   static ReadOnlyIntGauge* TOTAL_USED;
> Since SumGauge now inherits from ReadOnlyIntGauge, I would leave this as th
Fixed in PS2.


http://gerrit.cloudera.org:8080/#/c/24485/1/be/src/util/memory-metrics.cc
File be/src/util/memory-metrics.cc:

http://gerrit.cloudera.org:8080/#/c/24485/1/be/src/util/memory-metrics.cc@47
PS1, Line 47: ReadOnlyIntGauge* AggregateMemoryMetrics::TOTAL_USED = nullptr;
> See other comment about leaving this as a SumGauge type
Fixed in PS2.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ida124aa3225ee07cf30e9f0a481477474a50c4dd
Gerrit-Change-Number: 24485
Gerrit-PatchSet: 1
Gerrit-Owner: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Comment-Date: Mon, 29 Jun 2026 11:54:16 +0000
Gerrit-HasComments: Yes

Reply via email to