Marton Greber has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21723 )

Change subject: Add Prometheus HTTP service discovery
......................................................................


Patch Set 10:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/21723/9//COMMIT_MSG@110
PS9, Line 110: _sd_configs:
> What are you thoughts on multi-master Kudu clusters?  IIRC, the list of reg
Yep, that’s a good point—thanks for bringing it up.
As we saw with the REST API, 307 redirects can be tricky. With Prometheus 
there’s a neat workaround, if we include all 3 masters in the scrape config:
        •       the leader master returns the SD data, and
        •       the followers reply with HTTP 200 and an empty array.

Prometheus lets you provide multiple service-discovery configs:
"# List of HTTP service discovery configurations.
http_sd_configs:"

See the docs: 
https://prometheus.io/docs/prometheus/latest/configuration/configuration/#scrape_config


http://gerrit.cloudera.org:8080/#/c/21723/9/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:

http://gerrit.cloudera.org:8080/#/c/21723/9/src/kudu/master/master-test.cc@3709
PS9, Line 3709:   const char* kTableNameB = "testtableb";
> Did you find this test stable enough?  I'd think that in case of fast start
Yes—when I revisited this test I realized it was flaky and hard to stabilize, 
so I replaced it with a more deterministic approach in this revision.


http://gerrit.cloudera.org:8080/#/c/21723/9/src/kudu/master/master_path_handlers.cc
File src/kudu/master/master_path_handlers.cc:

http://gerrit.cloudera.org:8080/#/c/21723/9/src/kudu/master/master_path_handlers.cc@969
PS9, Line 969:   if (VLOG_IS_ON(1)) {
             :     auto refresh_interval_it = 
req.request_headers.find("X-Prometheus-Refresh-Interval-Seconds");
             :     if (refresh_interval_it != req.request_headers.end()) {
             :       VLOG(1) << Substitute("Prometheus service discovery 
refresh interval: $0 seconds",
             :
> nit: enclose this into 'if (VLOG_IS_ON(1)) { ... }' clause to avoid spendin
Done


http://gerrit.cloudera.org:8080/#/c/21723/9/src/kudu/master/master_path_handlers.cc@978
PS9, Line 978:   // should return service discovery data. Non-leader
> I think we should check for a stronger condition here, e.g. make sure the R
Done


http://gerrit.cloudera.org:8080/#/c/21723/9/src/kudu/master/master_path_handlers.cc@993
PS9, Line 993:   PrometheusSDData data;
> Shouldn't the server respond with HttpStatusCode::ServiceUnavailable here a
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I931aa72a7567c0dde43d7b7ed53a2dd0fa8bc1fe
Gerrit-Change-Number: 21723
Gerrit-PatchSet: 10
Gerrit-Owner: Marton Greber <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Gabriella Lotz <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Wang Xixu <[email protected]>
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Reviewer: Zoltan Martonka <[email protected]>
Gerrit-Comment-Date: Fri, 25 Jul 2025 13:25:25 +0000
Gerrit-HasComments: Yes

Reply via email to