Offloader metrics #13833

tjiuming · 2022-01-19T07:31:43Z

Motivation
Currently, there is no offload metrics for tiered storage, so it is very hard for us to debug the performance issues. For example , we can not find why offload is slow or why read offload is slow. For above reasons. we need to add some offload metrics for monitoring.

Modifications
Add metrics during offload procedure and read offload data procedure. Including offloadTime, offloadError, offloadRate, readLedgerLatency, writeStoreLatency, writeStoreError, readOffloadIndexLatency, readOffloadDataLatency, readOffloadRate, readOffloadError.

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

If yes was chosen, please highlight the changes

Dependencies (does it add or upgrade a dependency): (no)
The public API: (no)
The schema: (no)
The default values of configurations: (no)
The wire protocol: (no)
The rest endpoints: (no)
The admin cli options: (no)
Anything that affects deployment: (don't know)

Documentation

Check the box below or label this PR directly (if you have committer privilege).

Need to update docs?

doc-required

(If you need help on updating docs, create a doc issue)
no-need-doc

(Please explain why)
doc

(If this PR contains doc changes)

…oader-metrics

…oader-metrics-prometheus

…oader-metrics

tjiuming · 2022-01-19T07:33:36Z

TRB, tests will be complete later.

michaeljmarshall · 2022-01-20T05:23:31Z

Motivation
Currently, there is no offload metrics for tiered storage, so it is very hard for us to debug the performance issues. For example , we can not find why offload is slow or why read offload is slow. For above reasons. we need to add some offload metrics for monitoring.

Modifications
Add metrics during offload procedure and read offload data procedure. Including offloadTime, offloadError, offloadRate, readLedgerLatency, writeStoreLatency, writeStoreError, readOffloadIndexLatency, readOffloadDataLatency, readOffloadRate, readOffloadError.

@tjiuming - the content of this PR description is copied from #11555. Is the content of the PR also copied from that PR? Several committers had requested changes on that PR, so it'd be helpful to understand how this PR compares to #11555.

I see that @codelipenghui just closed that PR a couple days ago. @frankxieke, were you no longer planning on finishing #11555?

tjiuming · 2022-01-20T07:13:45Z

Motivation
Currently, there is no offload metrics for tiered storage, so it is very hard for us to debug the performance issues. For example , we can not find why offload is slow or why read offload is slow. For above reasons. we need to add some offload metrics for monitoring.
Modifications
Add metrics during offload procedure and read offload data procedure. Including offloadTime, offloadError, offloadRate, readLedgerLatency, writeStoreLatency, writeStoreError, readOffloadIndexLatency, readOffloadDataLatency, readOffloadRate, readOffloadError.

@tjiuming - the content of this PR description is copied from #11555. Is the content of the PR also copied from that PR? Several committers had requested changes on that PR, so it'd be helpful to understand how this PR compares to #11555.

I see that @codelipenghui just closed that PR a couple days ago. @frankxieke, were you no longer planning on finishing #11555?

#11555 is not finished, @frankxieke leaved streamnative, so I take over this issue and fix existing problems. @michaeljmarshall

…r-metrics

eolivelli

Overall the patch LGTM

but I left one comment about topics unloading, please take a look

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/LedgerOffloaderStatsImpl.java

eolivelli

Lgtm

lhotari · 2022-04-15T08:01:48Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java

        this.brokerClientSharedTimer =
                new HashedWheelTimer(new DefaultThreadFactory("broker-client-shared-timer"), 1, TimeUnit.MILLISECONDS);
+
+        int interval = config.getManagedLedgerStatsPeriodSeconds();


What if interval is set to <=0 ? Does that disable stats?

lhotari · 2022-04-15T08:08:23Z

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/LedgerOffloaderStats.java

+
+    static LedgerOffloaderStats create(boolean exposeManagedLedgerStats, boolean exposeTopicLevelMetrics,
+                                       ScheduledExecutorService scheduler, int interval) {
+        if (!exposeManagedLedgerStats) {


Should also contain the condition || interval <= 0

Generally speaking, the value of managedLedgerStatsPeriodSeconds always > 0(default value is 60000), but I can create another PR to fix this.

I think it makes sense to address in this PR.

Oh this was already merged, so please address in a new PR.

@tjiuming did you follow up on this comment ?

@eolivelli interval actually is managedLedgerStatsPeriodSeconds in ServiceConfiguration, and managedLedgerStatsPeriodSeconds is annotated by @FieldContext(minValue = 1). So, interval always >= 1

### Motivation Currently, there is no offload metrics for tiered storage, so it is very hard for us to debug the performance issues. For example , we can not find why offload is slow or why read offload is slow. For above reasons. we need to add some offload metrics for monitoring. ### Modifications Add metrics during offload procedure and read offload data procedure. Including offloadTime, offloadError, offloadRate, readLedgerLatency, writeStoreLatency, writeStoreError, readOffloadIndexLatency, readOffloadDataLatency, readOffloadRate, readOffloadError.

### Motivation Currently, there is no offload metrics for tiered storage, so it is very hard for us to debug the performance issues. For example , we can not find why offload is slow or why read offload is slow. For above reasons. we need to add some offload metrics for monitoring. ### Modifications Add metrics during offload procedure and read offload data procedure. Including offloadTime, offloadError, offloadRate, readLedgerLatency, writeStoreLatency, writeStoreError, readOffloadIndexLatency, readOffloadDataLatency, readOffloadRate, readOffloadError. (cherry picked from commit 732049f)

Currently, there is no offload metrics for tiered storage, so it is very hard for us to debug the performance issues. For example , we can not find why offload is slow or why read offload is slow. For above reasons. we need to add some offload metrics for monitoring. Add metrics during offload procedure and read offload data procedure. Including offloadTime, offloadError, offloadRate, readLedgerLatency, writeStoreLatency, writeStoreError, readOffloadIndexLatency, readOffloadDataLatency, readOffloadRate, readOffloadError. (cherry picked from commit 732049f)

### Motivation Currently, there is no offload metrics for tiered storage, so it is very hard for us to debug the performance issues. For example , we can not find why offload is slow or why read offload is slow. For above reasons. we need to add some offload metrics for monitoring. ### Modifications Add metrics during offload procedure and read offload data procedure. Including offloadTime, offloadError, offloadRate, readLedgerLatency, writeStoreLatency, writeStoreError, readOffloadIndexLatency, readOffloadDataLatency, readOffloadRate, readOffloadError. (cherry picked from commit 732049f)

Currently, there is no offload metrics for tiered storage, so it is very hard for us to debug the performance issues. For example , we can not find why offload is slow or why read offload is slow. For above reasons. we need to add some offload metrics for monitoring. Add metrics during offload procedure and read offload data procedure. Including offloadTime, offloadError, offloadRate, readLedgerLatency, writeStoreLatency, writeStoreError, readOffloadIndexLatency, readOffloadDataLatency, readOffloadRate, readOffloadError. (cherry picked from commit 732049f)

### Motivation Currently, there is no offload metrics for tiered storage, so it is very hard for us to debug the performance issues. For example , we can not find why offload is slow or why read offload is slow. For above reasons. we need to add some offload metrics for monitoring. ### Modifications Add metrics during offload procedure and read offload data procedure. Including offloadTime, offloadError, offloadRate, readLedgerLatency, writeStoreLatency, writeStoreError, readOffloadIndexLatency, readOffloadDataLatency, readOffloadRate, readOffloadError. (cherry picked from commit 732049f) Co-authored-by: Tao Jiuming <95597048+tjiuming@users.noreply.github.com>

* [ManagedLedger] Make 'StatsPeroidSeconds' configurable (apache#11584) Make `StatsPeroidSeconds` configurable. - Move ‘StatsPeriodSeconds’ from ManagedLedgerFactoryImpl to ManagedLedgerFactoryConfig. - Add config `managedLedgerStatsPeriodSeconds`. * Offloader metrics (apache#13833) Currently, there is no offload metrics for tiered storage, so it is very hard for us to debug the performance issues. For example , we can not find why offload is slow or why read offload is slow. For above reasons. we need to add some offload metrics for monitoring. Add metrics during offload procedure and read offload data procedure. Including offloadTime, offloadError, offloadRate, readLedgerLatency, writeStoreLatency, writeStoreError, readOffloadIndexLatency, readOffloadDataLatency, readOffloadRate, readOffloadError. (cherry picked from commit 732049f) Co-authored-by: GuoJiwei <technoboy@apache.org> Co-authored-by: Tao Jiuming <95597048+tjiuming@users.noreply.github.com>

* [ManagedLedger] Make 'StatsPeroidSeconds' configurable (apache#11584) Make `StatsPeroidSeconds` configurable. - Move ‘StatsPeriodSeconds’ from ManagedLedgerFactoryImpl to ManagedLedgerFactoryConfig. - Add config `managedLedgerStatsPeriodSeconds`. (cherry picked from commit ddeae65) * Offloader metrics (apache#13833) Currently, there is no offload metrics for tiered storage, so it is very hard for us to debug the performance issues. For example , we can not find why offload is slow or why read offload is slow. For above reasons. we need to add some offload metrics for monitoring. Add metrics during offload procedure and read offload data procedure. Including offloadTime, offloadError, offloadRate, readLedgerLatency, writeStoreLatency, writeStoreError, readOffloadIndexLatency, readOffloadDataLatency, readOffloadRate, readOffloadError. (cherry picked from commit 732049f) Co-authored-by: GuoJiwei <technoboy@apache.org> Co-authored-by: Tao Jiuming <95597048+tjiuming@users.noreply.github.com>

After #13833, we change the signature of LedgerOffloaderFactory#create. But some users may use the old version LedgerOffloaderFactory in the offloader nar, when pulsar load the offloader nar, it will use the LedgerOffloaderFactory in the offloader nar, the LedgerOffloaderFactory#create has no param offloaderStats. Modifications: Make LedgerOffloaderFactory can load the old nar.

ariestp316@gmail.com and others added 12 commits January 11, 2022 17:00

offload metrics

bbd8492

Merge branch 'master' of https://github.com/tjiuming/pulsar into offl…

0a1285b

…oader-metrics

offload metrics

95bb1bd

offload metrics

bc96d07

Merge branch 'master' of https://github.com/tjiuming/pulsar into offl…

bfbbce4

…oader-metrics-prometheus

Merge branch 'master' of https://github.com/tjiuming/pulsar into offl…

0bb1b75

…oader-metrics

offload metrics bug fix

3963402

Merge branch 'master' of https://github.com/tjiuming/pulsar into offl…

09a038e

…oader-metrics

offload metrics bug fix

55c798d

Merge branch 'master' of https://github.com/tjiuming/pulsar into offl…

0f38666

…oader-metrics

offload metrics bug fix

4bf32f4

Update OffloadPoliciesImpl.java

1a7bddd

github-actions bot assigned tjiuming Jan 19, 2022

github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Jan 19, 2022

codelipenghui added this to the 2.10.0 milestone Jan 19, 2022

codelipenghui added area/metrics area/tieredstorage labels Jan 19, 2022

codelipenghui requested review from 315157973, Technoboy-, congbobo184, gaoran10, hangc0276, hezhangjian, michaeljmarshall and zymap January 19, 2022 08:23

ariestp316@gmail.com added 2 commits January 20, 2022 20:55

offload metrics tests.

dc29bbb

Merge remote-tracking branch 'origin/offloader-metrics' into offloade…

df6bd24

…r-metrics

ariestp316@gmail.com added 2 commits April 13, 2022 11:15

review fix & resolve conflicts

380d830

make LedgerOffloaderStatsDisable's constructor private

c872fb9

eolivelli requested changes Apr 13, 2022

View reviewed changes

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/LedgerOffloaderStatsImpl.java Show resolved Hide resolved

managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/LedgerOffloaderStatsImpl.java Outdated Show resolved Hide resolved

review fix

7523a17

tjiuming requested a review from eolivelli April 13, 2022 07:45

checkstyle fix

6a81c88

eolivelli approved these changes Apr 13, 2022

View reviewed changes

codelipenghui approved these changes Apr 15, 2022

View reviewed changes

npe fix

579fc9d

codelipenghui merged commit 732049f into apache:master Apr 15, 2022

lhotari requested changes Apr 15, 2022

View reviewed changes

Anonymitaet added doc-complete Your PR changes impact docs and the related docs have been already added. and removed doc-required Your PR changes impact docs and you will update later. labels Apr 18, 2022

tjiuming deleted the offloader-metrics branch May 10, 2022 07:01

This was referenced Mar 23, 2023

[fix][broker]Make LedgerOffloaderFactory can load the old nar. #19913

Merged

[fix][broker]Make LedgerOffloaderFactory compatible with old version. horizonzy/pulsar#12

Closed

dao-jun mentioned this pull request Oct 17, 2024

[feat][monitor] Add offloader stats grafana dashboard #23479

Merged

15 tasks

Offloader metrics #13833

Offloader metrics #13833

Uh oh!

Conversation

tjiuming commented Jan 19, 2022

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

Documentation

Uh oh!

tjiuming commented Jan 19, 2022

Uh oh!

michaeljmarshall commented Jan 20, 2022

Uh oh!

tjiuming commented Jan 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eolivelli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

eolivelli 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

tjiuming commented Jan 20, 2022 •

edited

Loading