Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[monitor][broker][metadata] add metrics for BatchMetadataStore #17072

Merged
merged 16 commits into from
Nov 1, 2022
Merged

[monitor][broker][metadata] add metrics for BatchMetadataStore #17072

merged 16 commits into from
Nov 1, 2022

Conversation

tjiuming
Copy link
Contributor

@tjiuming tjiuming commented Aug 11, 2022 •
edited by momo-jun
Loading

Motivation

add metrics for batch_metadata_store.
This PR should being merged after #17041 in case of conflicts.

Documentation

Check the box below or label this PR directly.

Need to update docs?

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

Matching PR in forked repository

PR in forked repository: tjiuming#7

@github-actions github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Aug 11, 2022
@tjiuming tjiuming mentioned this pull request Aug 11, 2022
Merged
4 tasks
@codelipenghui codelipenghui added this to the 2.12.0 milestone Aug 13, 2022
@tjiuming tjiuming mentioned this pull request Aug 14, 2022
Merged
4 tasks
@Jason918
Copy link
Contributor

TIP: You can covert PR status to draft, if it's not ready for review. :)

@tjiuming tjiuming changed the title [DONT_MERGE][monitoring][broker][metadata_store] add metrics for BatchMetadataStore [draft][monitoring][broker][metadata_store] add metrics for BatchMetadataStore Aug 15, 2022
@tjiuming
Copy link
Contributor Author

TIP: You can covert PR status to draft, if it's not ready for review. :)

Thanks~

@Jason918
Copy link
Contributor

TIP: You can covert PR status to draft, if it's not ready for review. :)

Thanks~

See this.

image

tjiuming and others added 3 commits August 23, 2022 23:37
@tjiuming
Merge branch 'master' into dev/batch_metadata_store_stats
571a641
@dao-jun
Merge branch 'master' into dev/batch_metadata_store_stats
65f8f1f 
# Conflicts:
#	pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/AbstractMetadataStore.java
@dao-jun
fix
5d5759f
@tjiuming tjiuming changed the title [draft][monitoring][broker][metadata_store] add metrics for BatchMetadataStore [monitoring][broker][metadata_store] add metrics for BatchMetadataStore Sep 22, 2022
@tjiuming
Copy link
Contributor Author

@codelipenghui @asafm PTAL

asafm
asafm reviewed Sep 29, 2022
View reviewed changes
Copy link
Contributor

@asafm asafm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Mainly minor syntax comments aside from one main comment.

...ata/src/main/java/org/apache/pulsar/metadata/impl/batching/AbstractBatchedMetadataStore.java Outdated Show resolved Hide resolved
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/batching/OpPut.java Show resolved Hide resolved
...ar-metadata/src/main/java/org/apache/pulsar/metadata/impl/stats/BatchMetadataStoreStats.java Show resolved Hide resolved
...ar-metadata/src/main/java/org/apache/pulsar/metadata/impl/stats/BatchMetadataStoreStats.java Outdated Show resolved Hide resolved
...ar-metadata/src/main/java/org/apache/pulsar/metadata/impl/stats/BatchMetadataStoreStats.java Outdated Show resolved Hide resolved
pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/MetadataStoreStatsTest.java Outdated Show resolved Hide resolved
@tjiuming tjiuming mentioned this pull request Oct 24, 2022
Closed
@tjiuming tjiuming changed the title [monitoring][broker][metadata_store] add metrics for BatchMetadataStore [monitor][broker][metadata] add metrics for BatchMetadataStore Oct 24, 2022
@tjiuming
Copy link
Contributor Author

@asafm I've updated the PR and only keeps pulsar_batch_metadata_store_executor_queue_size, pulsar_batch_metadata_store_queue_wait_time.
and add pulsar_batch_metadata_store_batch_execute_time pulsar_batch_metadata_store_perbatch_ops

asafm
asafm approved these changes Oct 31, 2022
View reviewed changes
Copy link
Contributor

@asafm asafm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. One small comment left.

...ar-metadata/src/main/java/org/apache/pulsar/metadata/impl/stats/BatchMetadataStoreStats.java Outdated Show resolved Hide resolved
@tjiuming
Copy link
Contributor Author

@codelipenghui PTAL

@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

@codecov-commenter
Copy link

codecov-commenter commented Nov 1, 2022 •
edited
Loading

Codecov Report

Merging #17072 (5127d25) into master (0866c3a) will decrease coverage by 0.28%.
The diff coverage is 50.60%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #17072      +/-   ##
============================================
- Coverage     38.97%   38.69%   -0.29%     
+ Complexity     8311     8262      -49     
============================================
  Files           683      685       +2     
  Lines         67325    67335      +10     
  Branches       7217     7216       -1     
============================================
- Hits          26239    26053     -186     
- Misses        38079    38281     +202     
+ Partials       3007     3001       -6
Flag Coverage Δ
unittests 38.69% <50.60%> (-0.29%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../main/java/org/apache/pulsar/PulsarStandalone.java 0.00% <ø> (ø)
...pulsar/broker/service/PulsarCommandSenderImpl.java 74.86% <ø> (+0.51%) ⬆️
...ransaction/buffer/impl/InMemTransactionBuffer.java 0.00% <ø> (ø)
...nsaction/buffer/impl/TransactionBufferDisable.java 52.63% <ø> (ø)
.../metadata/v2/TransactionBufferSnapshotIndexes.java 0.00% <0.00%> (ø)
...a/v2/TransactionBufferSnapshotIndexesMetadata.java 0.00% <0.00%> (ø)
...oker/transaction/buffer/metadata/v2/TxnIDData.java 0.00% <0.00%> (ø)
...ulsar/client/impl/transaction/TransactionImpl.java 0.00% <0.00%> (ø)
...rg/apache/pulsar/client/util/ExecutorProvider.java 0.00% <0.00%> (ø)
...ransaction/buffer/impl/TopicTransactionBuffer.java 45.34% <42.85%> (+0.95%) ⬆️
... and 40 more

@tjiuming
Copy link
Contributor Author

tjiuming commented Nov 1, 2022

/pulsarbot run-failure-checks

@tjiuming
Copy link
Contributor Author

tjiuming commented Nov 1, 2022

@codelipenghui all the required checks are passed

@codelipenghui codelipenghui merged commit adae4ae into apache:master Nov 1, 2022
@tjiuming tjiuming deleted the dev/batch_metadata_store_stats branch November 1, 2022 17:37
@momo-jun
Copy link
Contributor

momo-jun commented Nov 2, 2022

@tjiuming did you have a plan to add some information about the new metrics?

@tjiuming
Copy link
Contributor Author

tjiuming commented Nov 2, 2022

@tjiuming did you have a plan to add some information about the new metrics?

there are already a PR for the doc, I'll update it soon

@labuladong labuladong mentioned this pull request Nov 2, 2022
Open
2 tasks
@momo-jun momo-jun 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 Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codelipenghui codelipenghui codelipenghui approved these changes

@asafm asafm asafm approved these changes

Assignees

@tjiuming tjiuming

Labels
area/metadata area/metrics doc-complete Your PR changes impact docs and the related docs have been already added. ready-to-test
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.
7 participants
@tjiuming @Jason918 @codelipenghui @codecov-commenter @momo-jun @asafm @dao-jun