Fix metrics Worker.ActiveClients counter error #14088

xichen01 · 2021-09-17T12:38:44Z

What changes are proposed in this pull request?

Fix metrics Worker.ActiveClients counter error

Why are the changes needed?

When executing end-to-end test: allxuio runTest --readType NO_CACHE --writeType THROUGH. the counter metric Worker.ActiveClients.xxx will change to 2 instead of 0.

Does this PR introduce any user facing changes?

No

alluxio-bot · 2021-09-17T12:38:45Z

Hi @xichen01, thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

alluxio-bot · 2021-09-17T12:38:49Z

Automated checks report:

Commits associated with Github account: FAIL
- It looks like your commits can't be linked to a valid Github account.
  Your commits are made with the email maritanchen@tencent.com, which does not allow your contribution to be tracked by Github.
  See this link for possible reasons this might be happening.
  To change the author email address that your most recent commit was made under, you can run:
```
git -c user.name="Name" -c user.email="Email" commit --amend --reset-author
```
  See this answer for more details about how to change commit email addresses.
  Once the author email address has been updated, update the pull request by running:
```
git push --force https://github.com/xichen01/alluxio.git fix-metrics-activeclient-error
```
PR title follows the conventions: PASS

Some checks failed. Please fix the reported issues and reply 'alluxio-bot, check this please' to re-run checks.

alluxio-bot · 2021-09-17T13:10:32Z

Automated checks report:

Commits associated with Github account: PASS
PR title follows the conventions: PASS

All checks passed!

apc999 · 2021-09-18T04:12:37Z

@xichen01 thanks for this fix! Really appreciate it as I introduced this metrics and glad to see you fixed it.
can you sign the CLA https://cla.alluxio.org/ ?

codecov-commenter · 2021-09-18T04:21:41Z

Codecov Report

Merging #14088 (49a54d5) into master (4a5801f) will decrease coverage by 0.08%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master   #14088      +/-   ##
============================================
- Coverage     43.28%   43.19%   -0.09%     
+ Complexity     9210     9195      -15     
============================================
  Files          1425     1425              
  Lines         82512    82518       +6     
  Branches       9983     9984       +1     
============================================
- Hits          35715    35645      -70     
- Misses        43832    43910      +78     
+ Partials       2965     2963       -2

Impacted Files	Coverage Δ
.../java/alluxio/worker/block/DefaultBlockWorker.java	`67.82% <100.00%> (+0.22%)`	⬆️
...lluxio/worker/block/UnderFileSystemBlockStore.java	`59.16% <100.00%> (+1.40%)`	⬆️
...a/alluxio/exception/status/CancelledException.java	`0.00% <0.00%> (-42.86%)`	⬇️
...rc/main/java/alluxio/worker/block/PinListSync.java	`76.92% <0.00%> (-23.08%)`	⬇️
...e/common/src/main/java/alluxio/AbstractClient.java	`59.86% <0.00%> (-12.50%)`	⬇️
...luxio/worker/block/meta/StorageDirEvictorView.java	`69.69% <0.00%> (-12.13%)`	⬇️
.../src/main/java/alluxio/retry/TimeBoundedRetry.java	`84.61% <0.00%> (-11.54%)`	⬇️
.../worker/block/management/tier/SwapRestoreTask.java	`62.83% <0.00%> (-10.62%)`	⬇️
...worker/block/management/BlockTransferExecutor.java	`87.93% <0.00%> (-6.90%)`	⬇️
...er/block/management/ManagementTaskCoordinator.java	`78.08% <0.00%> (-5.48%)`	⬇️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a5801f...49a54d5. Read the comment docs.

maobaolong · 2021-09-18T09:06:44Z

@xichen01 Can you sign the CLA https://cla.alluxio.org/ ?

alluxio-bot · 2021-09-18T11:24:42Z

You did it @xichen01!

Thank you for signing the Contribution License Agreement.

maobaolong · 2021-09-19T23:38:59Z

core/server/worker/src/main/java/alluxio/worker/block/DefaultBlockWorker.java

@@ -638,6 +638,13 @@ public void closeUfsBlock(long sessionId, long blockId)
          // the sessionId.
          LOG.debug("Invalid worker state while committing block.", e);
        }
+      } else {
+        // When getTempBlockMeta() return null, such as a block readType NO_CACHE writeType THROUGH.


@xichen01 Would you please show the cause why counter was not be decreased in the commitblock()?

Because the block metadata is not in memory (mLocalBlockStore.getTempBlockMeta() will return null). So, the branch commitblock() will not be executed.

alluxio/core/server/worker/src/main/java/alluxio/worker/block/DefaultBlockWorker.java

Lines 629 to 631 in 49a54d5

if (mLocalBlockStore.getTempBlockMeta(sessionId, blockId) != null) {

try {

commitBlock(sessionId, blockId, false);

Why block metadata is not in memory?
When executing the test case: allxuio runTest --readType NO_CACHE --writeType THROUGH. the counter metric

alluxio/core/server/worker/src/main/java/alluxio/worker/block/UnderFileSystemBlockReader.java

Lines 385 to 388 in fceea2a

if (mBlockWriter == null && offset == 0 && !mBlockMeta.isNoCache()) {

BlockStoreLocation loc = BlockStoreLocation.anyDirInTier(mStorageTierAssoc.getAlias(0));

mLocalBlockStore.createBlock(mBlockMeta.getSessionId(), mBlockMeta.getBlockId(),

AllocateOptions.forCreate(mInitialBlockSize, loc));

The block metadata will not update to memory, because is no_cache IO

yuzhu · 2021-09-27T23:03:07Z

@apc999 @maobaolong PTAL

yuzhu · 2021-10-13T06:03:23Z

@apc999 PTAL

maobaolong

It LGTM.

maobaolong · 2021-11-25T10:17:14Z

Hi @apc999 , PTAL, thank you.

xichen01 · 2022-01-24T03:01:43Z

Executing: allxuio runTest --readType NO_CACHE --writeType THROUGH.

As shown below, because the readType is NO_CACHE,so mLocalBlockStore.getTempBlockMeta() will return null. Therefore, the branch with commitBlock() will not be executed, so Metrics.WORKER_ACTIVE_CLIENTS.dec() in commitBlock() will not be executed, after the Worker Active Clients metric can exceed 0

example: executing: allxuio runTest --readType NO_CACHE --writeType THROUGH

ZhuTopher · 2022-01-25T02:13:08Z

alluxio-bot, check this please

alluxio-bot · 2022-01-25T02:13:12Z

Automated checks report:

Commits associated with Github account: PASS
PR title follows the conventions: PASS

All checks passed!

ZhuTopher · 2022-01-25T02:13:26Z

alluxio-bot, test this please

alluxio-bot · 2022-01-25T02:13:27Z

alluxio-bot triggering jenkins retest: 0/3

alluxio-bot · 2022-01-25T02:13:27Z

jenkins, test this please

yuzhu · 2022-01-25T04:54:13Z

alluxio-bot, merge this please

ZhuTopher · 2022-01-27T00:35:23Z

Mentioning that this PR fixes #14412 for posterity.

jiacheliu3 · 2022-01-30T01:46:09Z

@dbw9580 Is this needed for 2.7?

dbw9580 · 2022-01-30T01:56:22Z

Yes, I think backporting bug fixes is always desirable.

jiacheliu3 · 2022-01-30T02:07:00Z

alluxio-bot, cherry-pick this to branch-2.7 please

alluxio-bot · 2022-01-30T02:07:22Z

Auto cherry-pick to branch branch-2.7 successfully opened PR: #14941

### What changes are proposed in this pull request? Fix metrics Worker.ActiveClients counter error ### Why are the changes needed? When executing end-to-end test: allxuio runTest --readType NO_CACHE --writeType THROUGH. the counter metric Worker.ActiveClients.xxx will change to 2 instead of 0. ### Does this PR introduce any user facing changes? No pr-link: #14088 change-id: cid-9bfb67ba59896c03827a0bc8c84901f931a41ba6

…lluxio#14088 Fix metrics Worker.ActiveClients counter error Fix metrics Worker.ActiveClients counter error When executing end-to-end test: allxuio runTest --readType NO_CACHE --writeType THROUGH. the counter metric Worker.ActiveClients.xxx will change to 2 instead of 0. No pr-link: Alluxio#14088 change-id: cid-9bfb67ba59896c03827a0bc8c84901f931a41ba6

alluxio-bot added the cla-missing label Sep 17, 2021

Fix metrics Worker.ActiveClients counter error

49a54d5

xichen01 force-pushed the fix-metrics-activeclient-error branch from 91af1af to 49a54d5 Compare September 17, 2021 13:10

apc999 requested review from maobaolong and apc999 September 18, 2021 04:12

alluxio-bot removed the cla-missing label Sep 18, 2021

maobaolong reviewed Sep 19, 2021

View reviewed changes

alluxio-bot assigned ggezer Sep 20, 2021

yuzhu requested a review from maobaolong September 27, 2021 23:02

maobaolong approved these changes Nov 25, 2021

View reviewed changes

xichen01 mentioned this pull request Jan 24, 2022

Worker Active Clients metric can exceed 0 at rest #14412

Closed

yuzhu closed this Jan 25, 2022

yuzhu reopened this Jan 25, 2022

yuzhu approved these changes Jan 25, 2022

View reviewed changes

alluxio-bot merged commit 7b64a95 into Alluxio:master Jan 25, 2022

alluxio-bot mentioned this pull request Jan 30, 2022

Port changes of [#14088] to branch-2.7 #14941

Open

secfree mentioned this pull request Nov 8, 2022

Some worker metrics are not correct #16229

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix metrics Worker.ActiveClients counter error #14088

Fix metrics Worker.ActiveClients counter error #14088

xichen01 commented Sep 17, 2021 •

edited

Loading

alluxio-bot commented Sep 17, 2021

alluxio-bot commented Sep 17, 2021

alluxio-bot commented Sep 17, 2021

apc999 commented Sep 18, 2021

codecov-commenter commented Sep 18, 2021 •

edited

Loading

maobaolong commented Sep 18, 2021

alluxio-bot commented Sep 18, 2021

maobaolong Sep 19, 2021

xichen01 Sep 20, 2021

yuzhu commented Sep 27, 2021

yuzhu commented Oct 13, 2021

maobaolong left a comment

maobaolong commented Nov 25, 2021

xichen01 commented Jan 24, 2022

ZhuTopher commented Jan 25, 2022

alluxio-bot commented Jan 25, 2022

ZhuTopher commented Jan 25, 2022

alluxio-bot commented Jan 25, 2022

alluxio-bot commented Jan 25, 2022

yuzhu commented Jan 25, 2022

ZhuTopher commented Jan 27, 2022

jiacheliu3 commented Jan 30, 2022

dbw9580 commented Jan 30, 2022

jiacheliu3 commented Jan 30, 2022

alluxio-bot commented Jan 30, 2022

	if (mLocalBlockStore.getTempBlockMeta(sessionId, blockId) != null) {
	try {
	commitBlock(sessionId, blockId, false);

	if (mBlockWriter == null && offset == 0 && !mBlockMeta.isNoCache()) {
	BlockStoreLocation loc = BlockStoreLocation.anyDirInTier(mStorageTierAssoc.getAlias(0));
	mLocalBlockStore.createBlock(mBlockMeta.getSessionId(), mBlockMeta.getBlockId(),
	AllocateOptions.forCreate(mInitialBlockSize, loc));

Fix metrics Worker.ActiveClients counter error #14088

Fix metrics Worker.ActiveClients counter error #14088

Conversation

xichen01 commented Sep 17, 2021 • edited Loading

What changes are proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user facing changes?

alluxio-bot commented Sep 17, 2021

alluxio-bot commented Sep 17, 2021

alluxio-bot commented Sep 17, 2021

apc999 commented Sep 18, 2021

codecov-commenter commented Sep 18, 2021 • edited Loading

Codecov Report

maobaolong commented Sep 18, 2021

alluxio-bot commented Sep 18, 2021

maobaolong Sep 19, 2021

Choose a reason for hiding this comment

xichen01 Sep 20, 2021

Choose a reason for hiding this comment

yuzhu commented Sep 27, 2021

yuzhu commented Oct 13, 2021

maobaolong left a comment

Choose a reason for hiding this comment

maobaolong commented Nov 25, 2021

xichen01 commented Jan 24, 2022

ZhuTopher commented Jan 25, 2022

alluxio-bot commented Jan 25, 2022

ZhuTopher commented Jan 25, 2022

alluxio-bot commented Jan 25, 2022

alluxio-bot commented Jan 25, 2022

yuzhu commented Jan 25, 2022

ZhuTopher commented Jan 27, 2022

jiacheliu3 commented Jan 30, 2022

dbw9580 commented Jan 30, 2022

jiacheliu3 commented Jan 30, 2022

alluxio-bot commented Jan 30, 2022

xichen01 commented Sep 17, 2021 •

edited

Loading

codecov-commenter commented Sep 18, 2021 •

edited

Loading