Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kv: Support byte size pagination with intent resolution #77228

Closed
nvanbenschoten opened this issue Mar 1, 2022 · 2 comments · Fixed by #95141
Closed

kv: Support byte size pagination with intent resolution #77228

nvanbenschoten opened this issue Mar 1, 2022 · 2 comments · Fixed by #95141
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team

Comments

@nvanbenschoten
Copy link
Member

nvanbenschoten commented Mar 1, 2022

Intent resolution currently supports MaxSpanRequestKeys pagination, but not TargetBytes pagination. This means that if each intent is costly to resolve1, an intent resolution batch of 100-200 intents (100 for point resolution, 200 for ranged resolution) can exceed kv.raft.command.max_size.

To address this, we'll need to add byte size pagination to point and ranged intent resolution (ResolveIntentRequest and ResolveIntentRangeRequest).

We'll also need to add support for such pagination to requestbatcher, like we did in cf11645 with MaxKeysPerBatchReq for key pagination.

Jira issue: CRDB-13492

Epic CRDB-16159

Footnotes

  1. for instance, its value may need to be rewritten due to an updated timestamp,

@nvanbenschoten nvanbenschoten added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-kv-transactions Relating to MVCC and the transactional model. labels Mar 1, 2022
@erikgrinaker
Copy link
Contributor

Related to #77270.

@nvanbenschoten
Copy link
Member Author

When we address this, we should also do something about #77270.

@exalate-issue-sync exalate-issue-sync bot changed the title kv: support byte size pagination with intent resolution Support byte size pagination with intent resolution May 27, 2022
@exalate-issue-sync exalate-issue-sync bot added T-kv KV Team and removed A-kv-transactions Relating to MVCC and the transactional model. labels May 27, 2022
@nvanbenschoten nvanbenschoten changed the title Support byte size pagination with intent resolution kv: Support byte size pagination with intent resolution Sep 29, 2022
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Jan 5, 2023
In this PR, we add MVCCResolveWriteIntentOptions to
MVCCResolveWriteIntent and MVCCResolveWriteIntentRangeOptions to
MVCCResolveWriteIntentRange. Moreover, we additionally return numBytes
and resumeSpan in MVCCResolveWriteIntent and numBytes and resumeReason
in MVCCResolveWriteIntentRange, but these return values are currently
unused and serve as a placeholder in refactoring, but will be used in
the future.

Informs: cockroachdb#77228

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Jan 5, 2023
In this PR, we add MVCCResolveWriteIntentOptions to
MVCCResolveWriteIntent and MVCCResolveWriteIntentRangeOptions to
MVCCResolveWriteIntentRange. Moreover, we additionally return numBytes
and resumeSpan in MVCCResolveWriteIntent and numBytes and resumeReason
in MVCCResolveWriteIntentRange, but these return values are currently
unused and serve as a placeholder in refactoring, but will be used in
the future.

Informs: cockroachdb#77228

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Jan 6, 2023
In this PR, we add MVCCResolveWriteIntentOptions to
MVCCResolveWriteIntent and MVCCResolveWriteIntentRangeOptions to
MVCCResolveWriteIntentRange. Moreover, we additionally return numBytes
and resumeSpan in MVCCResolveWriteIntent and numBytes and resumeReason
in MVCCResolveWriteIntentRange, but these return values are currently
unused and serve as a placeholder in refactoring, but will be used in
the future.

Informs: cockroachdb#77228

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Jan 6, 2023
Informs: cockroachdb#77228

Intent resolution batches are sequenced on raft and each batch can
consist of 100-200 intents. If an intent key or even value in some cases
are large, it is possible that resolving all intents in the batch would
result in a raft command size exceeding the max raft command size
kv.raft.command.max_size.

To address this, we add support for TargetBytes in resolve intent and
resolve intent range commands, allowing us to stop resolving intents in
the batch as soon as we exceed the TargetBytes max bytes limit.

Adding support for byte size pagination for intent resolver and
RequestBatcher is added in PR cockroachdb#92268.

This PR adds pagination for asynchronous intent resolution. A future PR
will add pagination for synchronous intent resolution (i.e. EndTxn).

Release note (ops change): Added support for a byte limit on resolve
intent and resolve intent range raft commands to prevent such commands
from exceeding the max raft command size.
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Jan 6, 2023
Informs: cockroachdb#77228

Refactor key and byte pagination for the Get command into the MVCC layer
Previously, pagination was done in pkg/kv/kvserver/batcheval/cmd_get.go,
but to ensure consistency in where pagination logic is located across
all commands, we move the pagination logic for the Get command to the
MVCC layer where the pagination logic for most other commands is. This
also enables better parameter testing in the storage package since we
can leverage e.g. data-driven tests like TestMVCCHistories.

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Jan 6, 2023
Informs: cockroachdb#77228

Refactor key and byte pagination for the Get command into the MVCC layer
Previously, pagination was done in pkg/kv/kvserver/batcheval/cmd_get.go,
but to ensure consistency in where pagination logic is located across
all commands, we move the pagination logic for the Get command to the
MVCC layer where the pagination logic for most other commands is. This
also enables better parameter testing in the storage package since we
can leverage e.g. data-driven tests like TestMVCCHistories.

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Jan 30, 2023
Informs: cockroachdb#77228

Intent resolution batches are sequenced on raft and each batch can
consist of 100-200 intents. If an intent key or even value in some cases
are large, it is possible that resolving all intents in the batch would
result in a raft command size exceeding the max raft command size
kv.raft.command.max_size.

In PR cockroachdb#94814, we added support for TargetBytes for resolve intent and
resolve intent range raft commands.

In this PR, we set the TargetBytes field in the resolve intent and
resolve intent range requests in the RequestBatcher and IntentResolver
to 4MB. This allows us to stop resolving intents in the batch before we
get close to the max raft command size to lower the chance we exceed
this limit.

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Jan 30, 2023
Implement MVCCPagination which calls a user-inputted function f() until
it returns an error (note that the iterutil.StopIteration() error means
we have iterated through all elements), or until the number of keys hits
the maxKeys limit or the number of bytes hits the targetBytes limit.

MVCCPagination is a general framework that enables key and byte
pagination.

Informs: cockroachdb#77228

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Jan 30, 2023
In this PR, we push key pagination for EndTxn (currently located in
cmd_end_transaction.go) into the MVCC layer (mvcc.go) by integrating
MVCCPagination from the previous commit into the EndTxn command.

Informs: cockroachdb#77228

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Jan 30, 2023
Informs: cockroachdb#77228

Intent resolution batches are sequenced on raft and each batch can
consist of 100-200 intents. If an intent key or even value in some cases
are large, it is possible that resolving all intents in the batch would
result in a raft command size exceeding the max raft command size
kv.raft.command.max_size.

To address this, we add support for TargetBytes in resolve intent and
resolve intent range commands, allowing us to stop resolving intents in
the batch as soon as we exceed the TargetBytes max bytes limit.

This PR adds byte pagination for synchronous intent resolution (i.e.
the EndTxn / End Transaction command).

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Feb 22, 2023
Informs: cockroachdb#77228

Intent resolution batches are sequenced on raft and each batch can
consist of 100-200 intents. If an intent key or even value in some cases
are large, it is possible that resolving all intents in the batch would
result in a raft command size exceeding the max raft command size
kv.raft.command.max_size.

In PR cockroachdb#94814, we added support for TargetBytes for resolve intent and
resolve intent range raft commands.

In this PR, we set the TargetBytes field in the resolve intent and
resolve intent range requests in the RequestBatcher and IntentResolver
to 4MB. This allows us to stop resolving intents in the batch before we
get close to the max raft command size to lower the chance we exceed
this limit.

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Feb 22, 2023
Informs: cockroachdb#77228

Intent resolution batches are sequenced on raft and each batch can
consist of 100-200 intents. If an intent key or even value in some cases
are large, it is possible that resolving all intents in the batch would
result in a raft command size exceeding the max raft command size
kv.raft.command.max_size.

In PR cockroachdb#94814, we added support for TargetBytes for resolve intent and
resolve intent range raft commands.

In this PR, we set the TargetBytes field in the resolve intent and
resolve intent range requests in the RequestBatcher and IntentResolver
to 4MB. This allows us to stop resolving intents in the batch before we
get close to the max raft command size to lower the chance we exceed
this limit.

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Feb 23, 2023
Informs: cockroachdb#77228

Intent resolution batches are sequenced on raft and each batch can
consist of 100-200 intents. If an intent key or even value in some cases
are large, it is possible that resolving all intents in the batch would
result in a raft command size exceeding the max raft command size
kv.raft.command.max_size.

In PR cockroachdb#94814, we added support for TargetBytes for resolve intent and
resolve intent range raft commands.

In this PR, we set the TargetBytes field in the resolve intent and
resolve intent range requests in the RequestBatcher and IntentResolver
to 4MB. This allows us to stop resolving intents in the batch before we
get close to the max raft command size to lower the chance we exceed
this limit.

Release note: None
craig bot pushed a commit that referenced this issue Feb 23, 2023
92268: intentresolver: Add byte pagination to RequestBatcher and IntentResolver r=nvanbenschoten a=KaiSun314

Informs: #77228
    
Intent resolution batches are sequenced on raft and each batch can
consist of 100-200 intents. If an intent key or even value in some cases
are large, it is possible that resolving all intents in the batch would
result in a raft command size exceeding the max raft command size
kv.raft.command.max_size.

In PR #94814, we added support for TargetBytes for resolve intent and
resolve intent range raft commands.

In this PR, we set the TargetBytes field in the resolve intent and
resolve intent range requests in the RequestBatcher and IntentResolver
to 4MB. This allows us to stop resolving intents in the batch before we
get close to the max raft command size to lower the chance we exceed
this limit.

Release note: None

Co-authored-by: Kai Sun <[email protected]>
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Feb 23, 2023
Implement MVCCPagination which calls a user-inputted function f() until
it returns an error (note that the iterutil.StopIteration() error means
we have iterated through all elements), or until the number of keys hits
the maxKeys limit or the number of bytes hits the targetBytes limit.

MVCCPagination is a general framework that enables key and byte
pagination.

Informs: cockroachdb#77228

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Feb 23, 2023
In this PR, we push key pagination for EndTxn (currently located in
cmd_end_transaction.go) into the MVCC layer (mvcc.go) by integrating
MVCCPagination from the previous commit into the EndTxn command.

Informs: cockroachdb#77228

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Feb 24, 2023
In this PR, we push key pagination for EndTxn (currently located in
cmd_end_transaction.go) into the MVCC layer (mvcc.go) by integrating
MVCCPagination from the previous commit into the EndTxn command.

Informs: cockroachdb#77228

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Feb 24, 2023
Informs: cockroachdb#77228

Intent resolution batches are sequenced on raft and each batch can
consist of 100-200 intents. If an intent key or even value in some cases
are large, it is possible that resolving all intents in the batch would
result in a raft command size exceeding the max raft command size
kv.raft.command.max_size.

To address this, we add support for TargetBytes in resolve intent and
resolve intent range commands, allowing us to stop resolving intents in
the batch as soon as we exceed the TargetBytes max bytes limit.

This PR adds byte pagination for synchronous intent resolution (i.e.
the EndTxn / End Transaction command).

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Feb 25, 2023
Implement MVCCPagination which calls a user-inputted function f() until
it returns an error (note that the iterutil.StopIteration() error means
we have iterated through all elements), or until the number of keys hits
the maxKeys limit or the number of bytes hits the targetBytes limit.

MVCCPagination is a general framework that enables key and byte
pagination.

Informs: cockroachdb#77228

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Feb 25, 2023
In this PR, we push key pagination for EndTxn (currently located in
cmd_end_transaction.go) into the MVCC layer (mvcc.go) by integrating
MVCCPagination from the previous commit into the EndTxn command.

Informs: cockroachdb#77228

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Feb 25, 2023
Informs: cockroachdb#77228

Intent resolution batches are sequenced on raft and each batch can
consist of 100-200 intents. If an intent key or even value in some cases
are large, it is possible that resolving all intents in the batch would
result in a raft command size exceeding the max raft command size
kv.raft.command.max_size.

To address this, we add support for TargetBytes in resolve intent and
resolve intent range commands, allowing us to stop resolving intents in
the batch as soon as we exceed the TargetBytes max bytes limit.

This PR adds byte pagination for synchronous intent resolution (i.e.
the EndTxn / End Transaction command).

Release note: None
craig bot pushed a commit that referenced this issue Feb 26, 2023
94939: storage: Push pagination for EndTxn into the MVCC layer r=nvanbenschoten a=KaiSun314

Informs: #77228

This PR consists of two parts: adding `MVCCPagination` and integrating `MVCCPagination` to push pagination of `EndTxn` into the MVCC layer.

Part 1: Add `MVCCPagination`

Implement MVCCPagination which calls a user-inputted function f() until
it returns an error (note that the iterutil.StopIteration() error means
we have iterated through all elements), or until the number of keys hits
the maxKeys limit or the number of bytes hits the targetBytes limit.

MVCCPagination is a general framework that enables key and byte
pagination.

Part 2: Integrate `MVCCPagination` to Push Pagination of `EndTxn` Into The MVCC Layer

In this PR, we push key pagination for EndTxn (currently located in
cmd_end_transaction.go) into the MVCC layer (mvcc.go) by integrating
MVCCPagination from the previous commit into EndTxn.

Release note: None

Co-authored-by: Kai Sun <[email protected]>
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Feb 27, 2023
Informs: cockroachdb#77228

Intent resolution batches are sequenced on raft and each batch can
consist of 100-200 intents. If an intent key or even value in some cases
are large, it is possible that resolving all intents in the batch would
result in a raft command size exceeding the max raft command size
kv.raft.command.max_size.

To address this, we add support for TargetBytes in resolve intent and
resolve intent range commands, allowing us to stop resolving intents in
the batch as soon as we exceed the TargetBytes max bytes limit.

This PR adds byte pagination for synchronous intent resolution (i.e.
the EndTxn / End Transaction command).

Release note: None
KaiSun314 added a commit to KaiSun314/cockroach that referenced this issue Feb 27, 2023
Informs: cockroachdb#77228

Intent resolution batches are sequenced on raft and each batch can
consist of 100-200 intents. If an intent key or even value in some cases
are large, it is possible that resolving all intents in the batch would
result in a raft command size exceeding the max raft command size
kv.raft.command.max_size.

To address this, we add support for TargetBytes in resolve intent and
resolve intent range commands, allowing us to stop resolving intents in
the batch as soon as we exceed the TargetBytes max bytes limit.

This PR adds byte pagination for synchronous intent resolution (i.e. the
EndTxn / End Transaction command).

Release note: None
@craig craig bot closed this as completed in c3348b0 Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants