Skip to content

add bytelimit for prefetch (release-7.2)#8798

Merged
flowguru merged 2 commits into
apple:release-7.2from
flowguru:byte7
Nov 12, 2022
Merged

add bytelimit for prefetch (release-7.2)#8798
flowguru merged 2 commits into
apple:release-7.2from
flowguru:byte7

Conversation

@flowguru

@flowguru flowguru commented Nov 11, 2022

Copy link
Copy Markdown
Contributor

backport #8768

Code-Reviewer Section

The general pull request 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)

@flowguru
add bytelimit for prefetch (apple#8768)
46a2009 
* add bytelimit for prefetch

A fraction of byteLimit will be used as the limit to fetch index.
For the indexes fetched, fetch records for them in batch.

byteLimit always count the index size, it also count record if exist,
it at least return 1 index-record entry and always include the last entry
despite that adding the last entry despite it might exceed limit.

There is a Knob STRICTLY_ENFORCE_BYTE_LIMIT, when it is set, records
will be discarded once the byteLimit is hit, despite they are fetched.
Otherwise, return the whole batch.
jzhou77
jzhou77 reviewed Nov 11, 2022
View reviewed changes
Comment thread fdbserver/workloads/GetMappedRange.actor.cpp
}

// reset it to default
(const_cast<ServerKnobs*> SERVER_KNOBS)->STRICTLY_ENFORCE_BYTE_LIMIT = false;

@jzhou77 jzhou77 Nov 11, 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.

It's better to save the default value and restore from that, because the default may not be false (e.g., after a change) and then this is wrong.

@flowguru flowguru Nov 11, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sounds good, will have another PR for this change

halfprice
halfprice approved these changes Nov 11, 2022
View reviewed changes
Comment thread fdbclient/ServerKnobs.cpp
init( QUICK_GET_VALUE_FALLBACK, true );
init( QUICK_GET_KEY_VALUES_FALLBACK, true );
init( MAX_PARALLEL_QUICK_GET_VALUE, 50 ); if ( randomize && BUGGIFY ) MAX_PARALLEL_QUICK_GET_VALUE = deterministicRandom()->randomInt(1, 100);
init( STRICTLY_ENFORCE_BYTE_LIMIT, false); if( randomize && BUGGIFY ) STRICTLY_ENFORCE_BYTE_LIMIT = deterministicRandom()->coinflip() ? false : true;

@halfprice halfprice Nov 11, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit, it's better to make it aligned with other knobs.

@flowguru flowguru Nov 11, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is aligned in my IDE, and also called clang-format, not sure what is going on here..

@jzhou77 jzhou77 Nov 11, 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.

clang-format was turned off for this block.

@flowguru flowguru Nov 11, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see, will fix

@flowguru flowguru Nov 11, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#8800

@foundationdb-ci

foundationdb-ci commented Nov 11, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-clang on Linux CentOS 7

@foundationdb-ci

foundationdb-ci commented Nov 11, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux CentOS 7

  • Commit ID: 46a2009
  • Duration 1:13:14
  • Result: ❌ FAILED
  • Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
  • Build Logs (available for 30 days)
  • Build Artifact (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Nov 11, 2022

Copy link
Copy Markdown
Contributor

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

@flowguru

flowguru commented Nov 11, 2022

Copy link
Copy Markdown
Contributor Author 
TestFile="/var/joshua/ensembles/20221111-220310-pr8798-46a20091-1537-0327df2f3ebbf6a8/tests/rare/LargeApiCorrectnessStatus.toml

not related

@flowguru flowguru closed this Nov 11, 2022
@flowguru flowguru reopened this Nov 11, 2022
@foundationdb-ci

foundationdb-ci commented Nov 11, 2022

Copy link
Copy Markdown
Contributor

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

  • Commit ID: 46a2009
  • Duration 0:31:49
  • Result: ❌ FAILED
  • Error: Error while executing command: docker push ${DEV_REGISTRY}/foundationdb/foundationdb-kubernetes-sidecar:${FDB_VERSION}-${COMMIT_SHA}-1. Reason: exit status 1
  • Build Logs (available for 30 days)
  • Build Artifact (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-clang on Linux CentOS 7

  • Commit ID: 46a2009
  • Duration 0:49:00
  • Result: ❌ FAILED
  • Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
  • Build Logs (available for 30 days)
  • Build Artifact (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux CentOS 7

@fdb-windows-ci

fdb-windows-ci commented Nov 12, 2022

Copy link
Copy Markdown
Collaborator

Doxense CI Report for Windows 10

@flowguru

flowguru commented Nov 12, 2022

Copy link
Copy Markdown
Contributor Author 
TestFile="/var/joshua/ensembles/20221111-233713-pr8798-clang-46a20091-1525-cd1a35568d758db5/tests/restarting/from_7.1.0/SnapTestAttrition-1.txt

not related

@flowguru flowguru closed this Nov 12, 2022
@flowguru flowguru reopened this Nov 12, 2022
@foundationdb-ci

foundationdb-ci commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

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

  • Commit ID: 46a2009
  • Duration 0:32:16
  • Result: ❌ FAILED
  • Error: Error while executing command: docker push ${DEV_REGISTRY}/foundationdb/foundationdb-kubernetes-sidecar:${FDB_VERSION}-${COMMIT_SHA}-1. Reason: exit status 1
  • Build Logs (available for 30 days)
  • Build Artifact (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-macos-m1 on macOS BigSur 11.5.2

@foundationdb-ci

foundationdb-ci commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-clang on Linux CentOS 7

  • Commit ID: 46a2009
  • Duration 0:47:56
  • Result: ❌ FAILED
  • Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
  • Build Logs (available for 30 days)
  • Build Artifact (available for 30 days)

@foundationdb-ci

foundationdb-ci commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux CentOS 7

@flowguru

flowguru commented Nov 12, 2022

Copy link
Copy Markdown
Contributor Author

still unrelated joshua failure

@foundationdb-ci

foundationdb-ci commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-macos-m1 on macOS BigSur 11.5.2

@foundationdb-ci

foundationdb-ci commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-macos on macOS Monterey 12.x

@foundationdb-ci

foundationdb-ci commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-clang on Linux CentOS 7

@foundationdb-ci

foundationdb-ci commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux CentOS 7

@flowguru
flowguru merged commit 62970b3 into apple:release-7.2 Nov 12, 2022
@foundationdb-ci

foundationdb-ci commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

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

@fdb-windows-ci

fdb-windows-ci commented Nov 12, 2022

Copy link
Copy Markdown
Collaborator

Doxense CI Report for Windows 10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jzhou77 jzhou77 jzhou77 approved these changes
+1 more reviewer
@halfprice halfprice halfprice approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@flowguru @foundationdb-ci @fdb-windows-ci @halfprice @jzhou77