Set min replicate to 0 and unpin file if we fail to replicate a file #17865

yuzhu · 2023-07-31T21:11:05Z

What changes are proposed in this pull request?

Now we unpin and set min replication to 0 if we failed due to IO related reasons. (such as UFS not available, file does not exist etc).

Why are the changes needed?

Previously if a replication job fails, it would be rescheduled again and again, causing Denial of Service.

Does this PR introduce any user facing changes?

Yes, the user would expect certain files unpinned or adjusted if it becomes unavailable.

yuzhu · 2023-07-31T21:11:58Z

@jja725 some background context, a user saw repeated replication jobs because the file has been removed from the ufs, and was still pinned in alluxio.

jja725

Mostly LGTM

jja725 · 2023-07-31T21:39:32Z

job/server/src/main/java/alluxio/job/plan/replicate/SetReplicaDefinition.java

+    try {
+      JobUtils.loadBlock(status, context.getFsContext(), config.getBlockId(), null, false);
+    } catch (IOException e) {
+      LOG.warn("Replication of {} failed, reduce min replication to 1 and unpin.",


Suggested change

LOG.warn("Replication of {} failed, reduce min replication to 1 and unpin.",

LOG.warn("Replication of {} failed, reduce min replication to 0 and unpin.",

apc999 · 2023-07-31T21:52:50Z

job/server/src/main/java/alluxio/job/plan/replicate/SetReplicaDefinition.java

+          status.getPath());
+      SetAttributePOptions.Builder optionsBuilder =
+          SetAttributePOptions.newBuilder();
+      context.getFileSystem().setAttribute(new AlluxioURI(config.getPath()),


will this throw exception? how do we handle it?

addressed as a supressed exception

alluxio-bot · 2023-07-31T22:13:58Z

Automated checks report:

PR title follows the conventions: FAIL
- The title of the PR does not pass all the checks. Please fix the following issues:
  - Title must not end with punctuation
  - First word of title ("Unpin") is not an imperative verb. Please use one of the valid words
Commits associated with Github account: PASS

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

apc999 · 2023-07-31T22:44:18Z

job/server/src/main/java/alluxio/job/plan/replicate/SetReplicaDefinition.java

+        context.getFileSystem().setAttribute(new AlluxioURI(config.getPath()),
+            optionsBuilder.setReplicationMin(0).setPinned(false).build());
+      } catch (Throwable e2) {
+        e.addSuppressed(e2);


add LOG.warn here so at least we can find this suppressed issues in logs

apc999 · 2023-07-31T22:45:17Z

job/server/src/main/java/alluxio/job/plan/replicate/SetReplicaDefinition.java

+    try {
+      JobUtils.loadBlock(status, context.getFsContext(), config.getBlockId(), null, false);
+    } catch (IOException e) {
+      LOG.warn("Replication of {} failed, reduce min replication to 0 and unpin.",


add the failure reason?

jja725

LGTM

yuzhu · 2023-08-01T00:23:26Z

alluxio-bot, check this please

alluxio-bot · 2023-08-01T00:23:29Z

Automated checks report:

PR title follows the conventions: FAIL
- The title of the PR does not pass all the checks. Please fix the following issues:
  - Title must not end with punctuation
Commits associated with Github account: PASS

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

alluxio-bot · 2023-08-01T00:23:56Z

Automated checks report:

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

All checks passed!

yuzhu · 2023-08-01T00:24:12Z

alluxio-bot, merge this please

alluxio-bot · 2023-08-01T00:24:13Z

merge failed:
Merge refused because pull request does not have label start with type-

yuzhu · 2023-08-01T00:25:59Z

alluxio-bot, merge this please

yuzhu · 2023-08-01T00:26:27Z

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

alluxio-bot · 2023-08-01T00:26:55Z

Auto cherry-pick to branch branch-2.10 successfully opened PR: #17867

### What changes are proposed in this pull request? Now we unpin and set min replication to 0 if we failed due to IO related reasons. (such as UFS not available, file does not exist etc). ### Why are the changes needed? Previously if a replication job fails, it would be rescheduled again and again, causing Denial of Service. ### Does this PR introduce any user facing changes? Yes, the user would expect certain files unpinned or adjusted if it becomes unavailable. pr-link: #17865 change-id: cid-a2cde6da1e7a95fb361157307bd026303c0164ad

Cherry-pick of existing commit. orig-pr: #17865 orig-commit: c600830 orig-commit-author: David Zhu <david@alluxio.com> pr-link: #17867 change-id: cid-a2cde6da1e7a95fb361157307bd026303c0164ad

### What changes are proposed in this pull request? Merge missing commits from master-2.x to main. The commits in 2023/07/01~2023/11/08 from main...master-2.x will be included by this PR. We do this merge to catch missing fixes from `master-2.x` and catch the train before `main` cuts a release. #17747 is not cherry picked because tencent cloud EMR doc is removed #17755 is not cherry picked because DistLoadCliRunner has been removed in 3.x #17758 is not cherry picked because MonoBlockStore has been removed in 3.x #17641 is not cherry picked because the PR has already been in main #17781 is not cherry picked because the PR has already been in main #17722 is not cherry picked because the alluxio-fuse command has been changed a lot #17489 is not cherry picked because audit log on master is no longer in 3.x #17865 is not cherry picked because replication on job service is no longer in 3.x #17858 is not cherry picked because it is already in main #18090 is not cherry picked because generate-tarball has been rewritten in 3.x #18091 is not cherry picked because the change is already in main #17474 is not cherry picked because reconfiguration feature is not defined in 3.x #17735 is not cherry picked because MonoBlockStore is no longer in 3.x #18133 is not cherry picked because the issue is about master metadata and no longer relevant in 3.x #17910 is not cherry picked because I prefer to do that manually #17983 is not cherry picked because the web UI has been reworked #17984 is not cherry picked because Mount/Unmount commands have been reworked in 3.x #18103 is not cherry picked because worker cache metrics have been reworked in 3.x #18185 is not cherry picked because the report command has been reworked in 3.x #18222 is not cherry picked because Mount/Unmount operations have been reworked in 3.x #18143 is not cherry picked because the change is already in main #18303 is not cherry picked because the change is already in main #18208 is not cherry picked because cache metrics have been reworked in 3.x #17002 is not cherry picked because the owner has been notified separately #18334 is not cherry picked because the bash scripts have been reworked in 3.x #18326 is not cherry picked because the owner has been notified separately pr-link: #18397 change-id: cid-dbf8cbb2d9e721a5a0a1e5028a3c9577438a2ac0

Unpin a file and set min replicate to 0 if we fail to replicate a file.

b51e59c

yuzhu requested a review from jja725 July 31, 2023 21:11

jja725 requested changes Jul 31, 2023

View reviewed changes

apc999 reviewed Jul 31, 2023

View reviewed changes

Address comment

cc4b64f

yuzhu changed the title ~~Unpin a file and set min replicate to 0 if we fail to replicate a file.~~ Set min replicate to 0 and unpin file if we fail to replicate a file. Jul 31, 2023

apc999 reviewed Jul 31, 2023

View reviewed changes

Address comment

9af7a94

jja725 self-requested a review July 31, 2023 23:07

jja725 approved these changes Jul 31, 2023

View reviewed changes

Add a comment on the side effect

f46d5c1

yuzhu changed the title ~~Set min replicate to 0 and unpin file if we fail to replicate a file.~~ Set min replicate to 0 and unpin file if we fail to replicate a file Aug 1, 2023

yuzhu added the type-bug This issue is about a bug label Aug 1, 2023

alluxio-bot merged commit c600830 into Alluxio:master-2.x Aug 1, 2023
17 checks passed

alluxio-bot mentioned this pull request Aug 1, 2023

Set min replicate to 0 and unpin file i...; Port [#17865] to branch-2.10 #17867

Merged

jiacheliu3 mentioned this pull request Nov 8, 2023

Merge master-2.x commits 2023/07/01~2023/11/08 into main #18397

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set min replicate to 0 and unpin file if we fail to replicate a file #17865

Set min replicate to 0 and unpin file if we fail to replicate a file #17865

yuzhu commented Jul 31, 2023

yuzhu commented Jul 31, 2023

jja725 left a comment

jja725 Jul 31, 2023

apc999 Jul 31, 2023

yuzhu Jul 31, 2023

alluxio-bot commented Jul 31, 2023

apc999 Jul 31, 2023 •

edited

Loading

yuzhu Jul 31, 2023

apc999 Jul 31, 2023

yuzhu Jul 31, 2023

jja725 left a comment

yuzhu commented Aug 1, 2023

alluxio-bot commented Aug 1, 2023

alluxio-bot commented Aug 1, 2023

yuzhu commented Aug 1, 2023

alluxio-bot commented Aug 1, 2023

yuzhu commented Aug 1, 2023

yuzhu commented Aug 1, 2023

alluxio-bot commented Aug 1, 2023

	LOG.warn("Replication of {} failed, reduce min replication to 1 and unpin.",
	LOG.warn("Replication of {} failed, reduce min replication to 0 and unpin.",

Set min replicate to 0 and unpin file if we fail to replicate a file #17865

Set min replicate to 0 and unpin file if we fail to replicate a file #17865

Conversation

yuzhu commented Jul 31, 2023

What changes are proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user facing changes?

yuzhu commented Jul 31, 2023

jja725 left a comment

Choose a reason for hiding this comment

jja725 Jul 31, 2023

Choose a reason for hiding this comment

apc999 Jul 31, 2023

Choose a reason for hiding this comment

yuzhu Jul 31, 2023

Choose a reason for hiding this comment

alluxio-bot commented Jul 31, 2023

apc999 Jul 31, 2023 • edited Loading

Choose a reason for hiding this comment

yuzhu Jul 31, 2023

Choose a reason for hiding this comment

apc999 Jul 31, 2023

Choose a reason for hiding this comment

yuzhu Jul 31, 2023

Choose a reason for hiding this comment

jja725 left a comment

Choose a reason for hiding this comment

yuzhu commented Aug 1, 2023

alluxio-bot commented Aug 1, 2023

alluxio-bot commented Aug 1, 2023

yuzhu commented Aug 1, 2023

alluxio-bot commented Aug 1, 2023

yuzhu commented Aug 1, 2023

yuzhu commented Aug 1, 2023

alluxio-bot commented Aug 1, 2023

apc999 Jul 31, 2023 •

edited

Loading