incubator-uniffle icon indicating copy to clipboard operation
incubator-uniffle copied to clipboard

[MINOR] feat(server): Introduce metrics related bitmaps(committedBlockIds/cachedBlockIds/partitionToBlockIds)

Open maobaolong opened this issue 1 year ago • 5 comments

What changes were proposed in this pull request?

Introduce metrics related bitmaps.

Why are the changes needed?

Insight the block bitmap related count.

Fix: # (issue)

Does this PR introduce any user-facing change?

  • committed_block_count
  • reported_block_count
  • cached_block_count

How was this patch tested?

Locally.

image

maobaolong avatar Oct 15 '24 09:10 maobaolong

Test Results

 2 926 files  +21   2 926 suites  +21   5h 56m 58s :stopwatch: + 9m 5s  1 041 tests ± 0   1 039 :white_check_mark: + 1   2 :zzz: ±0  0 :x:  - 1  13 003 runs  +50  12 973 :white_check_mark: +51  30 :zzz: ±0  0 :x:  - 1 

Results for commit df4bbd01. ± Comparison against base commit a08c2272.

github-actions[bot] avatar Oct 15 '24 09:10 github-actions[bot]

@zuston Would you like to take a look at this PR? Thank you!

maobaolong avatar Oct 18 '24 01:10 maobaolong

@xianjingfeng @jerqi Look forward to your review, thanks!

maobaolong avatar Oct 22 '24 11:10 maobaolong

Will this pull request be affected by https://github.com/apache/incubator-uniffle/pull/2196?

jerqi avatar Oct 22 '24 12:10 jerqi

@jerqi yes, this will conflict if merge #2196, but doesn't matter, I will resolve it

maobaolong avatar Oct 22 '24 13:10 maobaolong

@jerqi Sorry for the mistake reply, these metrics can show the effect of #2196.

For our scenario, we use LOCAL_MEMORY storage type, so the committed_block_count and cached_block_count is 0 after combine these two PRs. I tested on our internal cluster.

maobaolong avatar Oct 23 '24 01:10 maobaolong

@jerqi Sorry for the mistake reply, these metrics can show the effect of #2196.

For our scenario, we use LOCAL_MEMORY storage type, so the committed_block_count and cached_block_count is 0 after combine these two PRs. I tested on our internal cluster.

LGTM if it won't throw an error.

jerqi avatar Oct 23 '24 11:10 jerqi

@jerqi yeah, it do not throw exception

maobaolong avatar Oct 23 '24 14:10 maobaolong