kvserver: add WriteIntentError byte limits
#72563
Labels
A-kv-server
Relating to the KV-level RPC server
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
T-kv
KV Team
Comments
erikgrinaker
added
C-enhancement
craig bot
pushed a commit
that referenced
this issue
Nov 25, 2021
72085: kvserver: add MVCC-compliant `AddSSTable` variant r=dt,tbg,sumeerbhola,nvanbenschoten a=erikgrinaker **storage: tweak `ScanIntents()`** This renames `ScanIntents` from `ScanSeparatedIntents`, and adds a context parameter. Callers have been updated to pass `storage.mvcc.max_intents_per_error` for the intent limit, as is done elsewhere, and `targetBytes` is passed as 0 for now (no limit) since other intent collectors currently don't use a byte limit. Touches #72563. Release note: None **roachpb: assert request flag combinations** Request flags have implicit dependencies and incompatibilities (e.g. `isLocking` implies `isTxn`). However, these were never checked and developers were expected to satisfy them manually, which is error-prone. This patch adds `TestFlagCombinations` that checks these dependencies and incompatibilities, based on `flagDependencies` and `flagExclusions` maps which encodes them. It also adds a new `flag` type for flags, renames `skipLeaseCheck` to `skipsLeaseCheck`, and adds `isAlone` for `CheckConsistencyRequest`. Release note: None **roachpb: add `appliesTSCache` request flag** Previously, `isIntentWrite` determined whether a request checked the timestamp cache and possibly pushed its timestamp. However, some requests may want to check the timestamp cache without writing intents, notably an MVCC-compliant `AddSSTable` request. This patch introduces a new flag `appliesTSCache`, and uses it as a condition for applying the timestamp cache to the request. Release note: None **kvserver: add MVCC-compliant `AddSSTable` variant** `AddSSTable` does not comply with MVCC, the timestamp cache, nor the closed timestamp, since the SST MVCC timestamps are written exactly as given and thus can rewrite history. This patch adds three new parameters that can make `AddSSTable` fully MVCC-compliant, with a corresponding `MVCCAddSSTable` version gate: * `WriteAtRequestTimestamp`: rewrites the MVCC timestamps to the request timestamp, complying with the timestamp cache and closed timestamp. * `DisallowConflicts`: checks for any conflicting keys and intents at or above the SST's MVCC timestamps, complying with MVCC. * `DisallowShadowingBelow`: implies `DisallowConflicts`, and also errors if shadowing visible keys (but not tombstones). Unlike the existing `DisallowShadowing`, this allows shadowing existing keys above the given timestamp if the new key has the same value as the existing one, and also allows idempotent writes at or above the given timestamp. The existing `DisallowShadowing` parameter implies `DisallowConflicts`, and also errors on any existing visible keys below the SST key's timestamp (but not tombstones). It no longer allows replacing a tombstone with a value at the exact same timestamp. Additionally, even blind `AddSSTable` requests that do not check for conflicts now take out lock spans and scan for existing intents, returning a `WriteIntentError` to resolve them. This should be cheap in the common case, since the caller is expected to ensure there are no concurrent writes over the span, and so there should be no or few intents. The `WriteAtRequestTimestamp` SST rewrite implementation here is correct but slow. Optimizations will be explored later. Resolves #70422. Release note: None Co-authored-by: Erik Grinaker <[email protected]>
|
We have marked this issue as stale because it has been inactive for |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We currently have a setting
storage.mvcc.max_intents_per_errorthat limits the number of intents included inWriteIntentErrorduring e.g. scans and exports. This defaults to 5000, which is fairly large, so we should consider an additional byte limit to avoid OOM situtations when encountering large intents.Jira issue: CRDB-11210
The text was updated successfully, but these errors were encountered: