[feat] [broker] Add broker health check status into prometheus metrics by vineeth1995 · Pull Request #10 · vineeth1995/pulsar

vineeth1995 · 2023-04-17T21:44:04Z

Motivation

To add broker health check into prometheus metric.

Modifications

Schedule a job at 1 minute time interval which calls healthCheck API on broker and updates the pulsar stats based on the broker health.

Verifying this change

Unit test cases were added to verify this change.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

Documentation

doc
doc-required
doc-not-needed
doc-complete

rdhabalia

let's create an issue first before creating this PR.
We want at least 10 entries for you in your contribution list to start vote for committer
https://github.com/apache/pulsar/pulls?q=is%3Apr+assignee%3Avineeth1995

rdhabalia · 2023-04-17T23:31:21Z

conf/broker.conf


+includeHealthCheckInMetrics=true
+healthCheckFrequencyInSeconds=60
+healthCheckInitialDelayInSecs=60


instead, all 3 config variables just keep one variable: healthCheckMetricsUpdateTimeInSeconds=-1. here -1 means disable. and please add config documentation in config.

rdhabalia · 2023-04-17T23:31:59Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

-import org.apache.pulsar.common.naming.SystemTopicNames;
-import org.apache.pulsar.common.naming.TopicDomain;
-import org.apache.pulsar.common.naming.TopicName;
+import org.apache.pulsar.common.naming.*;


having * is not good. please update with the required imports.

rdhabalia · 2023-04-17T23:37:24Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

                .numThreads(1)
                .build();
+
+        this.healthChecker = OrderedScheduler.newSchedulerBuilder()


initialize executor only if healthCheckMetricsUpdateTimeInSeconds >0 . so, just call method

void initializeHealthChecker() { if(healthCheckMetricsUpdateTimeInSeconds > 0){ healthChecker = new ... healthChecker.scheduleAtFixedRate(this::checkHealth, healthCheckMetricsUpdateTimeInSeconds, healthCheckMetricsUpdateTimeInSeconds, TimeUnit.SECONDS); } }

rdhabalia · 2023-04-17T23:37:42Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

        updateRates();
    }

+    protected void startHealthChecker() {


replaced by initializeHealthChecker(..)

rdhabalia · 2023-04-17T23:38:38Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/BrokerOperabilityMetrics.java

    private final LongAdder connectionCreateFailCount;
    private final LongAdder connectionTotalClosedCount;
    private final LongAdder connectionActive;
+    private volatile Long healthCheckStatus;


default value -1 means unknown 1 = success 0 = fail

so, if no one enables this task then value should be -1 unknown.

rdhabalia · 2023-04-19T21:34:42Z

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java

+            minValue = -1,
+            doc = "HealthCheck update frequency in seconds. Disable health check with value -1 (Default value 60)"
+    )
+    private int healthCheckMetricsUpdateTimeInSeconds = 60;


default should be disabled -1

rdhabalia · 2023-04-19T21:36:03Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

+                    .numThreads(1)
+                    .build();
+            int healthCheckFreqInSecs = config.getHealthCheckMetricsUpdateTimeInSeconds();
+            int healthCheckInitialDelayInSecs = config.getHealthCheckMetricsUpdateTimeInSeconds();


healthCheckFreqInSecs and healthCheckInitialDelayInSecs are same variable. we should use only 1 healthCheckFreqInSecs. please remove duplicate.

rdhabalia

LGTM

rdhabalia · 2023-04-20T00:24:08Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

+    }
+
+    public void checkHealth() {
+        BrokersBase.internalRunHealthCheck(TopicVersion.V2, pulsar(), null).thenAccept(__ -> {


import static import org.apache.pulsar.broker.admin.impl.BrokersBase.internalRunHealthCheck; internalRunHealthCheck(TopicVersion.V2, pulsar(), null)...

rdhabalia · 2023-04-20T00:24:54Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/BrokerOperabilityMetrics.java

    }
+
+    public void recordHealthCheckStatusSuccess() {
+        System.out.println("recording health check success");


remove System.out.println

rdhabalia · 2023-04-20T00:25:23Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/BrokerOperabilityMetrics.java

    private final LongAdder connectionCreateFailCount;
    private final LongAdder connectionTotalClosedCount;
    private final LongAdder connectionActive;
+    private volatile Long healthCheckStatus;


can we please add comment:
// 1=success, 0=failure, -1=unknown

github-actions · 2023-05-25T02:15:42Z

The pr had no activity for 30 days, mark with Stale label.

…yntax (apache#22867)" (apache#22921)

…, LeastLongTermMessageRate, ModularLoadManagerImpl. (apache#22889) Implementation PR: apache#22888 ### Motivation Initially, we introduce `loadBalancerCPUResourceWeight`, `loadBalancerBandwidthInResourceWeight`, `loadBalancerBandwidthOutResourceWeight`, `loadBalancerMemoryResourceWeight`, `loadBalancerDirectMemoryResourceWeight` in `ThresholdShedder` to control the resource weight for different resources when calculating the load of the broker. Then we let it work for `LeastResourceUsageWithWeight` for better bundle placement policy. But apache#19559 and apache#21168 have point out that the actual load of the broker is not related to the memory usage and direct memory usage, thus we have changed the default value of `loadBalancerMemoryResourceWeight`, `loadBalancerDirectMemoryResourceWeight` to 0.0. There are still some places where memory usage and direct memory usage are used to calculate the load of the broker, such as `OverloadShedder`, `LeastLongTermMessageRate`, `ModularLoadManagerImpl`. We should let the resource weight work for these places so that we can set the resource weight to 0.0 to avoid the impact of memory usage and direct memory usage on the load of the broker. ### Modifications - Let resource weight work for `OverloadShedder`, `LeastLongTermMessageRate`, `ModularLoadManagerImpl`.

### Motivation Those bundles that are filtered when try to unload them should not be included in the indicator. ### Modifications Increment the metric only when the bundle are unloaded.

…r Function runtimes (apache#22910)

…calling getPartitionedTopicMetadata (apache#22838)

…e#22924)

…vice doesn't get closed (apache#22858)

…tensibleLoadManagerImpl only) (apache#22930)

…pache#22931)

…he#22932)

…evel replication (apache#22890)

…, LeastLongTermMessageRate, ModularLoadManagerImpl. (apache#22888)

…rect add-opens parameters (apache#22927)

… position (apache#22856)

…gBitmap (apache#22908)

…th Apache Pulsar Helm chart (apache#23362)

…#23373)

…me (apache#23371)

…he#23377)

…m topic (ExtensibleLoadManagerImpl only) (apache#23381)

…hared implementation (apache#23219)

…recovery (apache#9292)

apache#23384)

…rary (apache#23375)

…ConsumerSelector (apache#23327)

…k and epoch time (apache#23345)

…he#22913)

…egistry (ExtensibleLoadManagerImpl only) (apache#23382)

Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Matteo Merli <mmerli@apache.org>

…3394)

…ble errors (apache#21759)

…he#22938)

…pache#23397)

…chema ledger (apache#23395)

…ge Ordering (apache#23309)

vineeth1995 force-pushed the healthcheck branch from d4fea23 to a86f44f Compare April 17, 2023 21:57

rdhabalia reviewed Apr 17, 2023

View reviewed changes

vineeth1995 force-pushed the healthcheck branch 3 times, most recently from 89dcf3c to 3f9b152 Compare April 19, 2023 21:31

rdhabalia reviewed Apr 19, 2023

View reviewed changes

vineeth1995 force-pushed the healthcheck branch from 930f240 to e929a12 Compare April 20, 2023 00:20

rdhabalia approved these changes Apr 20, 2023

View reviewed changes

vineeth1995 force-pushed the healthcheck branch 3 times, most recently from f74cf57 to 8216576 Compare April 24, 2023 20:19

github-actions bot added the Stale label May 25, 2023

coderzc and others added 17 commits June 17, 2024 11:25

[fix] Revert "[fix][cli] Fix the shell script parameter passthrough s…

fa74538

…yntax (apache#22867)" (apache#22921)

[fix] [broker] fix unload bundle count metric. (apache#22895)

dfbf05a

### Motivation Those bundles that are filtered when try to unload them should not be included in the indicator. ### Modifications Increment the metric only when the bundle are unloaded.

[improve][misc] Bump RoaringBitmap version to 1.0.6 (apache#22920)

7020ea2

[fix][fn] Enable optimized Netty direct byte buffer support for Pulsa…

f3d4d5a

…r Function runtimes (apache#22910)

[fix] [broker] response not-found error if topic does not exist when …

9aed736

…calling getPartitionedTopicMetadata (apache#22838)

[fix][test] Fix TableViewBuilderImplTest NPE and infinite loop (apach…

2dc0d96

…e#22924)

[fix] [client] Fix resource leak in Pulsar Client since HttpLookupSer…

bc3dc77

…vice doesn't get closed (apache#22858)

[fix][broker] Update init and shutdown time and other minor logic (Ex…

aa8f696

…tensibleLoadManagerImpl only) (apache#22930)

[improve][cli] Use LoadManagerReport instead of Object (apache#22850)

5ed07c1

[feat][broker] PIP-264: Add OpenTelemetry broker connection metrics (a…

6a1bbe6

…pache#22931)

[cleanup][client] Remove unneeded sync scope in TransactionImpl (apac…

bacb162

…he#22932)

[fix] [broker] Messages lost on the remote cluster when using topic l…

feae589

…evel replication (apache#22890)

[improve] [pip] PIP-358: let resource weight work for OverloadShedder…

5dc0304

…, LeastLongTermMessageRate, ModularLoadManagerImpl. (apache#22888)

[fix][cli] Refactor scripts to detect major Java version and pass cor…

e0f545a

…rect add-opens parameters (apache#22927)

[improve] [broker] PIP-356 Support Geo-Replication starts at earliest…

5fc0eaf

… position (apache#22856)

[improve][broker] Optimize ConcurrentOpenLongPairRangeSet by Roarin…

5b1f653

…gBitmap (apache#22908)

Demogorgon314 and others added 25 commits September 29, 2024 15:13

[fix][test] Fix ReplicatorRateLimiterTest (apache#23369)

5e832a1

[fix][cli] Remove deprecated "-client" JVM arg (apache#23370)

f071a89

[improve] Install openssl in the docker image to fix compatibility wi…

7e59bde

…th Apache Pulsar Helm chart (apache#23362)

[improve][ci] Switch to Java 21 as default JVM version for CI (apache…

b242850

…#23373)

[fix][broker] Fix the broker registering might be blocked for long ti…

7d7dc80

…me (apache#23371)

[improve] Upgrade Pulsar Python client in docker image to 3.5.0 (apac…

9980967

…he#23377)

[improve][broker] check system topic is used before configuring syste…

e0b754d

…m topic (ExtensibleLoadManagerImpl only) (apache#23381)

[feat] Use producer name and sequence number as fallback key in Key_S…

5b98d37

…hared implementation (apache#23219)

[fix][broker] Support large number of unack message store for cursor …

9eeffe5

…recovery (apache#9292)

[fix][broker] Cancel possible pending replay read in cancelPendingRead (

d2c91b1

apache#23384)

[improve][pip] PIP-381: Handle large PositionInfo state (apache#23328)

50802be

[improve][pip] Improve PIP process and voting timeline (apache#23387)

1dad078

[improve] Configure Rocksdb to use musl libc flavor of the native lib…

c41c7e9

…rary (apache#23375)

[fix][broker] Fix out-of-order issues with ConsistentHashingStickyKey…

adb9014

…ConsumerSelector (apache#23327)

[fix][client] Fix failover consumer-listener stuck with cumulative ac…

53e996c

…k and epoch time (apache#23345)

[improve][pip] PIP-360 Add admin API to display Schema metadata (apac…

e49d9ad

…he#22913)

[fix][sec] Upgrade protobuf-java to 3.25.5 (apache#23356)

ab684a0

[fix][broker] timeout when broker registry hangs and monitor broker r…

eee9283

…egistry (ExtensibleLoadManagerImpl only) (apache#23382)

[fix] Bump commons-io:commons-io from 2.8.0 to 2.14.0 (apache#23393)

c2128dc

Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Matteo Merli <mmerli@apache.org>

[fix][sec] Upgrade Avro to 1.11.4 to address CVE-2024-47561 (apache#2…

fad6761

…3394)

[improve][broker] PIP-327: Support force topic loading for unrecovera…

38322a6

…ble errors (apache#21759)

[improve][pip] PIP-360 Add admin API to display Schema metadata (apac…

1e93677

…he#22938)

[improve][ci] Continue Pulsar CI build even when Trivy scanner fails (a…

56200aa

…pache#23397)

[fix][broker] Fix Broker was failing to create producer with broken s…

aa12561

…chema ledger (apache#23395)

[improve][pip] PIP-379: Key_Shared Draining Hashes for Improved Messa…

64e9687

…ge Ordering (apache#23309)

vineeth1995 force-pushed the healthcheck branch 3 times, most recently from 50bb381 to 29f77e9 Compare October 4, 2024 23:13

[feat] [broker] Add broker health check status into prometheus metrics

84c4a3e

vineeth1995 force-pushed the healthcheck branch from 29f77e9 to 84c4a3e Compare October 5, 2024 06:30

Conversation

vineeth1995 commented Apr 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Uh oh!

rdhabalia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rdhabalia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

vineeth1995 commented Apr 17, 2023 •

edited

Loading