Skip to content

[DD Testability] Add unittest for resuming infligh relocations#7621

Merged
sfc-gh-xwang merged 8 commits into
apple:mainfrom
sfc-gh-xwang:feature/dd-refactor-incremental
Jul 24, 2022
Merged

[DD Testability] Add unittest for resuming infligh relocations#7621
sfc-gh-xwang merged 8 commits into
apple:mainfrom
sfc-gh-xwang:feature/dd-refactor-incremental

Conversation

@sfc-gh-xwang

@sfc-gh-xwang sfc-gh-xwang commented Jul 19, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Add test case "/DataDistributor/Initialization/ResumeFromShard".
There will be an OOM if the number of shards reaches 1 million. So we should limit the shard number also in practice.

Code-Reviewer Section

The general guidelines can be found here.

Please check each of the following things and check all boxes before accepting a PR.

  • The PR has a description, explaining both the problem and the solution.
  • The description mentions which forms of testing were done and the testing seems reasonable.
  • Every function/class/actor that was touched is reasonably well documented.

For Release-Branches

If this PR is made against a release-branch, please also check the following:

  • This change/bugfix is a cherry-pick from the next younger branch (younger release-branch or main if this is the youngest branch)
  • There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)

@fdb-windows-ci

fdb-windows-ci commented Jul 19, 2022

Copy link
Copy Markdown
Collaborator

Doxense CI Report for Windows 10

@foundationdb-ci

foundationdb-ci commented Jul 19, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux CentOS 7

  • Commit ID: 1c543e3
  • Duration 1:04:51
  • Result: ✅ SUCCEEDED
  • Error: N/A
  • Build Logs (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Jul 19, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

  • Commit ID: 43099bd
  • Duration 0:18:44
  • Result: ❌ FAILED
  • Error: Error while executing command: ninja -v -C build_output -j ${NPROC} all packages strip_targets. Reason: exit status 1
  • Build Logs (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Jul 19, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux CentOS 7

  • Commit ID: 43099bd
  • Duration 0:19:40
  • Result: ❌ FAILED
  • Error: Error while executing command: ninja -v -C build_output -j ${NPROC} all packages strip_targets. Reason: exit status 1
  • Build Logs (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Jul 19, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

  • Commit ID: 1c543e3
  • Duration 2:24:06
  • Result: ❌ FAILED
  • Error: Error while executing command: make -C tests -kj run. Reason: exit status 2
  • Build Logs (available for 30 days)

@fdb-windows-ci

fdb-windows-ci commented Jul 19, 2022

Copy link
Copy Markdown
Collaborator

Doxense CI Report for Windows 10

@foundationdb-ci

foundationdb-ci commented Jul 19, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux CentOS 7

  • Commit ID: a5fba60
  • Duration 1:04:43
  • Result: ✅ SUCCEEDED
  • Error: N/A
  • Build Logs (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Jul 19, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

  • Commit ID: a5fba60
  • Duration 2:28:22
  • Result: ❌ FAILED
  • Error: Error while executing command: make -C tests -kj run. Reason: exit status 2
  • Build Logs (available for 30 days)

@fdb-windows-ci

fdb-windows-ci commented Jul 20, 2022

Copy link
Copy Markdown
Collaborator

Doxense CI Report for Windows 10

@foundationdb-ci

foundationdb-ci commented Jul 20, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux CentOS 7

  • Commit ID: 4103250
  • Duration 1:03:06
  • Result: ✅ SUCCEEDED
  • Error: N/A
  • Build Logs (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Jul 21, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

  • Commit ID: 4103250
  • Duration 3:15:55
  • Result: ❌ FAILED
  • Error: Error while executing command: make -C tests -kj run. Reason: exit status 2
  • Build Logs (available for 30 days)

sfc-gh-bvr
sfc-gh-bvr reviewed Jul 21, 2022
View reviewed changes
Comment thread fdbserver/DataDistribution.actor.cpp Outdated
ACTOR static Future<Void> resumeRelocations(Reference<DataDistributor> self) {
ASSERT(self->shardsAffectedByTeamFailure); // has to be allocated

ACTOR static Future<Void> resumeFromInitShards(Reference<DataDistributor> self, bool traceShard) {

@sfc-gh-bvr sfc-gh-bvr Jul 21, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can the name be simpler, like setupShards?

Comment thread fdbserver/DataDistribution.actor.cpp Outdated
return Void();
}

ACTOR static Future<Void> resumeFromInitDataMoveMap(Reference<DataDistributor> self) {

@sfc-gh-bvr sfc-gh-bvr Jul 21, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same here - can the name simply be initDataMoveMap?

@sfc-gh-xwang sfc-gh-xwang Jul 21, 2022

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I want to retain the resumeFrom which describes the intention more accurately in my opinion.

@sfc-gh-xwang sfc-gh-xwang requested a review from sfc-gh-bvr July 21, 2022 16:19
@fdb-windows-ci

fdb-windows-ci commented Jul 21, 2022

Copy link
Copy Markdown
Collaborator

Doxense CI Report for Windows 10

@fdb-windows-ci

fdb-windows-ci commented Jul 21, 2022

Copy link
Copy Markdown
Collaborator

Doxense CI Report for Windows 10

@foundationdb-ci

foundationdb-ci commented Jul 21, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-macos on macOS BigSur 11.5.2

  • Commit ID: 7b8ea1d
  • Duration 0:42:44
  • Result: ✅ SUCCEEDED
  • Error: N/A
  • Build Logs (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Jul 21, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux CentOS 7

  • Commit ID: 0156996
  • Duration 1:05:48
  • Result: ✅ SUCCEEDED
  • Error: N/A
  • Build Logs (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Jul 21, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux CentOS 7

  • Commit ID: 7b8ea1d
  • Duration 1:13:28
  • Result: ✅ SUCCEEDED
  • Error: N/A
  • Build Logs (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Jul 21, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

  • Commit ID: 7b8ea1d
  • Duration 4:18:23
  • Result: ❌ FAILED
  • Error: Build has timed out.
  • Build Logs (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Jul 21, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

  • Commit ID: 0156996
  • Duration 4:16:42
  • Result: ❌ FAILED
  • Error: Build has timed out.
  • Build Logs (available for 30 days)

@sfc-gh-xwang sfc-gh-xwang merged commit 7b6f1ca into apple:main Jul 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sfc-gh-bvr sfc-gh-bvr sfc-gh-bvr approved these changes
@sfc-gh-tclinkenbeard sfc-gh-tclinkenbeard Awaiting requested review from sfc-gh-tclinkenbeard
@liquid-helium liquid-helium Awaiting requested review from liquid-helium

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sfc-gh-xwang @fdb-windows-ci @foundationdb-ci @sfc-gh-bvr