[kudu-CR] [WIP][catalog manager] Skip eviction during bootstrapping

2024-02-06 Thread Code Review
Ádám Bakai has abandoned this change. ( http://gerrit.cloudera.org:8080/20958 )

Change subject: [WIP][catalog_manager] Skip eviction during bootstrapping
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I0cbbe6b77d99d167fc58692047072f86d3d963cf
Gerrit-Change-Number: 20958
Gerrit-PatchSet: 6
Gerrit-Owner: Ádám Bakai 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Ádám Bakai 


[kudu-CR] [WIP][catalog manager] Skip eviction during bootstrapping

2024-02-06 Thread Code Review
Ádám Bakai has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20958 )

Change subject: [WIP][catalog_manager] Skip eviction during bootstrapping
..


Patch Set 6:

My root cause analysis was incorrect, I will abandon this PR and create a new 
one.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cbbe6b77d99d167fc58692047072f86d3d963cf
Gerrit-Change-Number: 20958
Gerrit-PatchSet: 6
Gerrit-Owner: Ádám Bakai 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Ádám Bakai 
Gerrit-Comment-Date: Tue, 06 Feb 2024 11:01:06 +
Gerrit-HasComments: No


[kudu-CR] KUDU-3549 fix WriteAsPrometheus() for non-arithmetic gauges

2024-02-06 Thread Marton Greber (Code Review)
Marton Greber has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20990 )

Change subject: KUDU-3549 fix WriteAsPrometheus() for non-arithmetic gauges
..


Patch Set 1: Code-Review+2

Did verify that the provided test fails without this fix.
Thanks for taking care of this issue!


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7128f52729c7f984004811153a7eecc8ffe751b
Gerrit-Change-Number: 20990
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Abhishek Chennaka 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber 
Gerrit-Reviewer: Yingchun Lai 
Gerrit-Comment-Date: Tue, 06 Feb 2024 13:07:27 +
Gerrit-HasComments: No


[kudu-CR] KUDU-3549 fix WriteAsPrometheus() for non-arithmetic gauges

2024-02-06 Thread Attila Bukor (Code Review)
Attila Bukor has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20990 )

Change subject: KUDU-3549 fix WriteAsPrometheus() for non-arithmetic gauges
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7128f52729c7f984004811153a7eecc8ffe751b
Gerrit-Change-Number: 20990
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Abhishek Chennaka 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber 
Gerrit-Reviewer: Yingchun Lai 
Gerrit-Comment-Date: Tue, 06 Feb 2024 13:15:29 +
Gerrit-HasComments: No


[kudu-CR] [rpc] validate security-related parameters earlier

2024-02-06 Thread Marton Greber (Code Review)
Marton Greber has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20961 )

Change subject: [rpc] validate security-related parameters earlier
..


Patch Set 1:

Would it makes sense to add a basic test, to verify that the verification works 
as intended?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie16537c440041c9ee74276da35f2599592ef7e04
Gerrit-Change-Number: 20961
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Abhishek Chennaka 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber 
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yingchun Lai 
Gerrit-Comment-Date: Tue, 06 Feb 2024 13:17:01 +
Gerrit-HasComments: No


[kudu-CR] KUDU-613: Scan Resistant Caching

2024-02-06 Thread Marton Greber (Code Review)
Marton Greber has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20607 )

Change subject: KUDU-613: Scan Resistant Caching
..


Patch Set 8:

(2 comments)

+1 on what Alexey was proposing, spliting up the changelist into smaller chunks.

http://gerrit.cloudera.org:8080/#/c/20607/8/src/kudu/util/block_cache_metrics.h
File src/kudu/util/block_cache_metrics.h:

PS8:
> Why is it necessary to update the block cache metrics in the same changelis
+1


http://gerrit.cloudera.org:8080/#/c/20607/8/src/kudu/util/cache.h
File src/kudu/util/cache.h:

http://gerrit.cloudera.org:8080/#/c/20607/8/src/kudu/util/cache.h@67
PS8, Line 67:   // Recency list cache implementations (FIFO, LRU, etc.)
> Is it possible to separate out this and other refactoring (i.e. moving dupl
+1



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45531534a2049dd38c002f4dc7982df9fd46e0bb
Gerrit-Change-Number: 20607
Gerrit-PatchSet: 8
Gerrit-Owner: Mahesh Reddy 
Gerrit-Reviewer: Abhishek Chennaka 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy 
Gerrit-Reviewer: Marton Greber 
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 06 Feb 2024 13:21:02 +
Gerrit-HasComments: Yes


[kudu-CR] [catalog manager] Tighten leader UUID fallback

2024-02-06 Thread Code Review
Ádám Bakai has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/21004


Change subject: [catalog_manager] Tighten leader UUID fallback
..

[catalog_manager] Tighten leader UUID fallback

It is safe to assume that if the term is the same in the current cstate
as in the previous cstate then even if the leader is not set, it will
be the same. But it is possible that cmeta file is deleted then
recreated with "local_replica cmeta unsafe_recreate" command. In this
case the leader_uuid is empty in the new cmeta file. This means that the
peer doesn't consider itself a leader, so no health report is generated
in tablet report and it has no leader_uuid set either. When a master
receives tablet report like this and there isn't a new term, then the
catalog master will treat this peer as a leader, but it will fail on a
check because the leader has to be in healthy status. This happened in
ToolTest::TestRecreateCMeta.

The solution is that catalog manager only assumes the previous leader
for the peer if the previous leader is not the peer itself. This gives
time for the peers to form a consensus about the leader.

Change-Id: I06a80a4a0a9fd422b50860e8cd8bf0e12973cd43
---
M src/kudu/master/catalog_manager.cc
1 file changed, 2 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/21004/1
--
To view, visit http://gerrit.cloudera.org:8080/21004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I06a80a4a0a9fd422b50860e8cd8bf0e12973cd43
Gerrit-Change-Number: 21004
Gerrit-PatchSet: 1
Gerrit-Owner: Ádám Bakai 


[kudu-CR] [log block manager] Write lock for deletion

2024-02-06 Thread Code Review
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins,

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

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

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

Change subject: [log_block_manager] Write lock for deletion
..

[log_block_manager] Write lock for deletion

Both removing blocks and compacting metadata manipulates the
live_blocks_ value and file content. This way it can happen that on
thread A live_blocks_ is updated, on thread B metadata is
compacted and live_blocks_ value is overwritten, then thread A writes
deletion records in the metadata file. This way it creates inconsistency
between file content and live_blocks_ value, which leads to incorrect
behaviour to skip compaction later.
This fix makes sure that CompactMetadata and deletion can not overlap
each other. No new test is needed because it fixes LogBlockManagerTest::
TestContainerBlockLimitingByMetadataSizeWithCompaction flakiness.

Change-Id: I73712a5fd8eccf23621f8abda3e99ebdf10a769f
---
M src/kudu/fs/log_block_manager.cc
1 file changed, 2 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/20901/12
--
To view, visit http://gerrit.cloudera.org:8080/20901
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I73712a5fd8eccf23621f8abda3e99ebdf10a769f
Gerrit-Change-Number: 20901
Gerrit-PatchSet: 12
Gerrit-Owner: Ádám Bakai 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] [log block manager-test] Improve random selection

2024-02-06 Thread Code Review
Hello Mahesh Reddy, Tidy Bot, Zoltan Martonka, Kudu Jenkins, Wang Xixu,

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

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

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

Change subject: [log_block_manager-test] Improve random selection
..

[log_block_manager-test] Improve random selection

The previous solution to remove random blocks was not good, because it
was possible that way less blocks are removed than expected because it
is possible that rand()%100 returns 0 or 99 every time and that means in
extreme cases zero or all the elements are deleted. The new approach
removes exactly the same number of blocks every time but randomizes
which blocks are removed and the order of removal is randomized, too.

Change-Id: I9f73f0572319784b07845b02d15a7d3e6f31abce
---
M src/kudu/fs/log_block_manager-test.cc
1 file changed, 5 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/20899/9
--
To view, visit http://gerrit.cloudera.org:8080/20899
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f73f0572319784b07845b02d15a7d3e6f31abce
Gerrit-Change-Number: 20899
Gerrit-PatchSet: 9
Gerrit-Owner: Ádám Bakai 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy 
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Zoltan Martonka 
Gerrit-Reviewer: Ádám Bakai 


[kudu-CR] KUDU-3549 fix WriteAsPrometheus() for non-arithmetic gauges

2024-02-06 Thread Alexey Serbin (Code Review)
Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20990 )

Change subject: KUDU-3549 fix WriteAsPrometheus() for non-arithmetic gauges
..

KUDU-3549 fix WriteAsPrometheus() for non-arithmetic gauges

This patch fixes WriteAsPrometheus() implementation for string-based
gauges.  The original changelist that introduced Prometheus metrics
had a proper implementation of WriteAsPrometheus() only for StringGauge,
but any FunctionGauge for a non-arithmetic type (e.g., for std::string)
would still output a string value in Prometheus text format, and that
could not be consumed by Prometheus.  The patch also contains a test
scenario that would fail without the fix.

This is a follow-up to 00efc6826ac9a1f5d10750296c7357790a041fec.

Change-Id: Ib7128f52729c7f984004811153a7eecc8ffe751b
Reviewed-on: http://gerrit.cloudera.org:8080/20990
Tested-by: Alexey Serbin 
Reviewed-by: Marton Greber 
Reviewed-by: Attila Bukor 
---
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
3 files changed, 90 insertions(+), 63 deletions(-)

Approvals:
  Alexey Serbin: Verified
  Marton Greber: Looks good to me, approved
  Attila Bukor: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib7128f52729c7f984004811153a7eecc8ffe751b
Gerrit-Change-Number: 20990
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Abhishek Chennaka 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber 
Gerrit-Reviewer: Yingchun Lai 


[kudu-CR] [log block manager-test] Improve random selection

2024-02-06 Thread Marton Greber (Code Review)
Marton Greber has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20899 )

Change subject: [log_block_manager-test] Improve random selection
..


Patch Set 9: Code-Review+1

LGTM, lets get a green build and I'll +2 it.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f73f0572319784b07845b02d15a7d3e6f31abce
Gerrit-Change-Number: 20899
Gerrit-PatchSet: 9
Gerrit-Owner: Ádám Bakai 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy 
Gerrit-Reviewer: Marton Greber 
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Zoltan Martonka 
Gerrit-Reviewer: Ádám Bakai 
Gerrit-Comment-Date: Tue, 06 Feb 2024 16:33:46 +
Gerrit-HasComments: No


[kudu-CR](branch-1.17.x) [test] a small clean-up on StringGaugePrometheusTest

2024-02-06 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/21006


Change subject: [test] a small clean-up on StringGaugePrometheusTest
..

[test] a small clean-up on StringGaugePrometheusTest

I took a quick look at KUDU-3549, and a natural first step was to check
whether we have a proper test coverage for handling StringGauge in when
dumping metrics in Prometheus format.  The result was this patch, adding
a couple new sub-scenarios into MetricsTest.StringGaugePrometheusTest.

While this didn't lead to discovering the root case of the reported
issue, the new sub-scenarios are still useful since they provide extra
coverage for the related functionality.  I also took the liberty of
cleaning up the code a bit.

Change-Id: Ie495f3bae91facd98e9ce61ee86f34dea8b08087
Reviewed-on: http://gerrit.cloudera.org:8080/20984
Reviewed-by: Yingchun Lai 
Tested-by: Alexey Serbin 
(cherry picked from commit ce8ef84d9cf1c4aca8edd07920dfef274edb8df4)
---
M src/kudu/util/metrics-test.cc
1 file changed, 43 insertions(+), 33 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/06/21006/1
--
To view, visit http://gerrit.cloudera.org:8080/21006
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie495f3bae91facd98e9ce61ee86f34dea8b08087
Gerrit-Change-Number: 21006
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 


[kudu-CR](branch-1.17.x) [test] a small clean-up on StringGaugePrometheusTest

2024-02-06 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21006 )

Change subject: [test] a small clean-up on StringGaugePrometheusTest
..


Patch Set 1: Verified+1

unrelated test failures


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie495f3bae91facd98e9ce61ee86f34dea8b08087
Gerrit-Change-Number: 21006
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Abhishek Chennaka 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai 
Gerrit-Comment-Date: Wed, 07 Feb 2024 00:44:35 +
Gerrit-HasComments: No


[kudu-CR](branch-1.17.x) [test] a small clean-up on StringGaugePrometheusTest

2024-02-06 Thread Alexey Serbin (Code Review)
Alexey Serbin has removed a vote on this change.

Change subject: [test] a small clean-up on StringGaugePrometheusTest
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/21006
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ie495f3bae91facd98e9ce61ee86f34dea8b08087
Gerrit-Change-Number: 21006
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Abhishek Chennaka 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai 


[kudu-CR] [catalog manager] Tighten leader UUID fallback

2024-02-06 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21004 )

Change subject: [catalog_manager] Tighten leader UUID fallback
..


Patch Set 1: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21004/1//COMMIT_MSG
Commit Message:

PS1:
Thank you for getting to the bottom of this!

Do you think it makes sense to add a new test or update the existing one 
(TestRecreateCMeta?) to make the issue happening more consistently when the fix 
is not present?  My concern is that KUDU-2335 was manifesting itself rarely, 
and if there is a regression in the code once it's updated, it would be hard to 
detect the issue right away.


http://gerrit.cloudera.org:8080/#/c/21004/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/21004/1/src/kudu/master/catalog_manager.cc@5456
PS1, Line 5456: // previous cstate, and the leader was known for that 
term.
Could you please add an extra comment, explaining the essence of the extra 
criterion added?

The extra comment might be something like below (but it's up to you to decide 
what's the best way to describe the rationale, of course):

An extra condition is to check whether it's a report from a former leader 
replica which currently doesn't maintain the leadership role.  Such a situation 
is possible when the replica's cmeta file had been deleted and then recreated 
(e.g. by the "kudu local_replica cmeta unsafe_recreate" CLI tool).  The code 
below assumes that the replica effectively has the leadership if the 
'leader_uuid' field is set, but that's not so (see KUDU-2335).



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06a80a4a0a9fd422b50860e8cd8bf0e12973cd43
Gerrit-Change-Number: 21004
Gerrit-PatchSet: 1
Gerrit-Owner: Ádám Bakai 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 07 Feb 2024 01:11:35 +
Gerrit-HasComments: Yes


[kudu-CR](branch-1.17.x) [test] a small clean-up on StringGaugePrometheusTest

2024-02-06 Thread Yingchun Lai (Code Review)
Yingchun Lai has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21006 )

Change subject: [test] a small clean-up on StringGaugePrometheusTest
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie495f3bae91facd98e9ce61ee86f34dea8b08087
Gerrit-Change-Number: 21006
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Abhishek Chennaka 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai 
Gerrit-Comment-Date: Wed, 07 Feb 2024 02:28:09 +
Gerrit-HasComments: No


[kudu-CR](branch-1.17.x) [test] a small clean-up on StringGaugePrometheusTest

2024-02-06 Thread Alexey Serbin (Code Review)
Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/21006 )

Change subject: [test] a small clean-up on StringGaugePrometheusTest
..

[test] a small clean-up on StringGaugePrometheusTest

I took a quick look at KUDU-3549, and a natural first step was to check
whether we have a proper test coverage for handling StringGauge in when
dumping metrics in Prometheus format.  The result was this patch, adding
a couple new sub-scenarios into MetricsTest.StringGaugePrometheusTest.

While this didn't lead to discovering the root case of the reported
issue, the new sub-scenarios are still useful since they provide extra
coverage for the related functionality.  I also took the liberty of
cleaning up the code a bit.

Change-Id: Ie495f3bae91facd98e9ce61ee86f34dea8b08087
Reviewed-on: http://gerrit.cloudera.org:8080/20984
Reviewed-by: Yingchun Lai 
Tested-by: Alexey Serbin 
(cherry picked from commit ce8ef84d9cf1c4aca8edd07920dfef274edb8df4)
Reviewed-on: http://gerrit.cloudera.org:8080/21006
---
M src/kudu/util/metrics-test.cc
1 file changed, 43 insertions(+), 33 deletions(-)

Approvals:
  Alexey Serbin: Verified
  Yingchun Lai: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie495f3bae91facd98e9ce61ee86f34dea8b08087
Gerrit-Change-Number: 21006
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Abhishek Chennaka 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai 


[kudu-CR](branch-1.17.x) KUDU-3549 fix WriteAsPrometheus() for non-arithmetic gauges

2024-02-06 Thread Alexey Serbin (Code Review)
Hello Marton Greber, Attila Bukor,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: KUDU-3549 fix WriteAsPrometheus() for non-arithmetic gauges
..

KUDU-3549 fix WriteAsPrometheus() for non-arithmetic gauges

This patch fixes WriteAsPrometheus() implementation for string-based
gauges.  The original changelist that introduced Prometheus metrics
had a proper implementation of WriteAsPrometheus() only for StringGauge,
but any FunctionGauge for a non-arithmetic type (e.g., for std::string)
would still output a string value in Prometheus text format, and that
could not be consumed by Prometheus.  The patch also contains a test
scenario that would fail without the fix.

This is a follow-up to 00efc6826ac9a1f5d10750296c7357790a041fec.

Change-Id: Ib7128f52729c7f984004811153a7eecc8ffe751b
Reviewed-on: http://gerrit.cloudera.org:8080/20990
Tested-by: Alexey Serbin 
Reviewed-by: Marton Greber 
Reviewed-by: Attila Bukor 
(cherry picked from commit 6b1c1eb0c97a2349e0b3fa098bf40f8147b43a60)
  Conflicts:
src/kudu/util/metrics.cc
src/kudu/util/metrics.h
---
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
3 files changed, 98 insertions(+), 71 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/21007/1
--
To view, visit http://gerrit.cloudera.org:8080/21007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib7128f52729c7f984004811153a7eecc8ffe751b
Gerrit-Change-Number: 21007
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Marton Greber