Conversation
Convert all 28 NativeAPI actors, remove the actorcompiler dependency, and rename the translation unit to NativeAPI.cpp. Preserve public signatures, lazy choose evaluation, retry state resets, and asynchronous ownership. Add regressions for transaction-state and metadata-promise release on completion and error paths. Validation on the unchanged source snapshot: - Managed Clang fdbclient_test and fdbserver builds passed. - NativeAPI: 5 passed; broader client tests: 40 normal, 41 simulated. - Watches, StreamingRangeRead, GetMappedRange, and ApiCorrectness simulations passed with buggify enabled (seeds 20260825-20260828).
Renaming NativeAPI to a standard C++ translation unit enables clang-tidy on this file. Address the newly checked diagnostics while preserving coroutine ownership, suspension ordering, and retry behavior.
Route getRawVersion and waitForCommittedVersionImpl through the GRV helper with proxy-change-first readiness semantics. Rebuild requests from live version-vector and tracing state on each retry. Cover readiness priority, request cleanup, and error propagation while preserving the existing helper default.
Remove the five GRV helper tests and the fixture, builder, and responder used only by those tests. Keep the GRV implementation and existing NativeAPI tests unchanged.
…e#13944) * BulkDump: keep the read failure that a bulkdump retry is hiding getRangeDataToDump collapses any failure of its first read into retry(), so the one distinction the caller acts on is lost. The SS bulkdump handler gives up immediately on wrong_shard_server -- a range assignment that has gone stale, which retrying against this server can never satisfy -- but it never sees that code, so the task instead spends its whole 50-attempt budget re-reading a range it does not own. Observed in simulation as exactly 51 SSBulkDumpError events carrying only error_code_retry, with no record anywhere of what actually failed. Rethrow the original error, and trace it: a retry that discards its own cause cannot be diagnosed from a trace log. * BulkDump: throw the read failure at its source Rethrowing from the catch handler makes the deferred firstReadError variable unnecessary: immediateError is cleared immediately after the try/catch, before every other break, so it can only still be set when the first read threw. Assert that invariant instead of carrying a fallback for a path that cannot occur.
…pple#13945) BULKDUMP_JOB_TIMEOUT and BULKLOAD_JOB_TIMEOUT are spent against total elapsed time, so both monitors abort jobs that are advancing normally. Their own comments concede the premise is wrong -- "24 hours - large DBs may take days" annotating a 24 hour budget -- because neither duration is a function of health. A dump advances one bounded slice per scheduling round: the storage server returns after SS_BULKDUMP_BATCH_COUNT_MAX_PER_REQUEST batches and the remainder is re-dispatched on a later round, so a range that begins life as a single shard needs far more rounds than the budget allows. A restore's duration likewise tracks the data it moves. Measured on a narrow-fleet simulation where the first location read returns one shard for the whole key space: seven seeds abort a dump that is demonstrably progressing -- 47 ranges persisted Complete, the frontier advancing across re-dispatch rounds -- and report a backup failure for it. Since apple#13936 that failure is no longer silent, which is what made this visible. Time out on absence of progress instead, keeping the knobs as the no-progress window so a genuinely wedged job is still caught. The restore monitor already computes the task counters it needs for its own status display. The dump monitor has no such counter, so it samples getBulkDumpCompleteTaskCount at a tenth of the window -- that call walks the job's metadata and cannot run at poll frequency -- and treats a failed sample as carrying no information, neither failing the backup nor extending its deadline. Seven previously failing seeds now complete the dump; twenty paired seeds show no regression.
Restore unknown_error diagnostics for unexpected exceptions in direct and deferred RPC delivery, reply serialization, and KAIO completion delivery. Keep typed errors and the default detached coroutine contract unchanged. Add native regressions for direct and deferred delivery, value and error reply serialization, and ordinary error reply behavior.
…rd/range-lock-owner-removal Prevent removing range-lock owners with active locks
…rd/docs-coroutines Remove legacy ACTOR code from documentation
…lid trace (apple#13940) The "AuditRequestInvalid" TraceEvent in serveAuditStorageRequests() adds .detail("AuditRange", req.range) twice. TraceEventFields::addField only appends fields (it does not deduplicate), so the event is serialized with the AuditRange key repeated, e.g. `AuditRange="..." ... AuditRange="..."`. Duplicate attribute names are invalid XML and produce a duplicate key in the JSON trace format, which breaks downstream trace-log parsing for every invalid audit request. Both occurrences use the identical value, so the second one carries no information. Drop it.
…rd/flow-tests-coroutines Convert FlowTests to standard coroutines
Added functionality for fdb.bash to support ipv6, provided the new env variable FDB_IP_VERSION is set to "6". Before, fdb.bash would throw an error if `hostname -i` resolved to a v6 address, as it would not be encased in brackets. Based on the value of FDB_IP_VERSION, fdb coordinator ip is discovered with either `getent ahostsv4` or `getent ahostsv6`. This will use the normal dns resolving path in the container, including local hosts file.
… be verified (apple#13873) A 10B validation lost 935,560 key-values while reporting success. Two problems, each hiding the other. An unhealthy destination team was acked unretryable, which marks the task Error and abandons it. A bulkload task's key-values exist only in the dump until an attempt ingests them, so that drops the range with no other copy. It is acked retryable and re-dispatched now, unpublished first because publishTask refuses an already-published taskId. DD_BULKLOAD_MAX_RETRYABLE_REDISPATCH bounds a task's total thrash; it is drawn once per process, since a per-dispatch buggify() made the effective budget the minimum over draws against a monotonic restartCount. monitorBulkLoadJobCompletionWithProgress read "job no longer running" as success, but a fully walked job clears its live metadata even when a task ended in Error. It now requires the archived phase to be Complete, the only phase attesting every task was ingested; Error, Cancelled and a missing entry all fail. An unreadable history fails too: the task counters are precisely what lied here, reporting 103786/103786 while data was missing, so they cannot license skipping the check. The cost is that an intact restore can fail on an unavailable read, which the trace states outright. Failure is terminal, so it aborts instead of throwing and staying retryable, which had it re-read the same finished job eleven times. The abort must also release the lock: ABORTED puts the restore outside isRunnable(), so abortRestore() returns before its unlockDatabase(). SevWarnAlways rather than SevError, because a task that could not be loaded is a property of the data, and SevError would fail any simulation provoking it. BULKLOAD_SIM_INJECT_DEST_TEAM_FAILURES injects a fixed count against one task; the path is too rare to reach by chance, and a probability either never fires or never stops. The three BackupS3Blob restore tests gain extraStorageMachineCountPerDC -- they had been restoring incomplete data and passing. Signed-off-by: neethuhaneesha
…rd/remove-orphaned-azure-backup-20260828 Remove orphaned Azure backup integration
Preserve ready-future caching and regression coverage across the upstream NativeAPI, FlowTests, and LoadBalance coroutine conversions.
…estore (apple#13923) * BulkLoad: split an unplaceable bulkload task instead of failing the restore A bulkload data move needs a destination team disjoint from src, and src is the union of the owners of every shard the task's range spans, so a range spread across enough of the fleet has no legal destination. Re-attempting cannot clear it: every attempt presents the same range and recomputes the same src. The relocator gave up at stuckCount == 50, the task was marked Error, and the restore failed. Measured on a 100M-record cluster: ValidTeamSize 0 with every candidate team rejected for source overlap, twice in a row. Narrow the task instead. Replace it with two tasks covering halves of its range, splitting the manifest list at the same key, and commit both writes in one transaction so no version exists in which the range is unowned or owned by anything but tasks whose union is the parent. Fewer shards per task means a smaller src, and at some granularity a disjoint team exists. Three things the first attempts got wrong, each worth keeping: - The children's ranges derive from the parent's range, not its manifests' span. A task's range is that span intersected with the job range, so deriving from manifest min/max hands a child key space the parent never had -- which tripped the tiling assertion on BulkDumpingS3WithChaos. - The cut must be a manifest boundary strictly inside the parent's range. A task clipped at a job-range edge holds manifests whose midpoint lies outside it, and cutting there leaves one child empty, so the split declined and handed back the very task it exists to rescue. - A split replaces one task with two, so a range may be owned by several tasks and a monitored task may be superseded by narrower ones. Neither is a job failure. The ack outcome is an exclusive enum rather than three bools; Unplaceable used to be a second flag whose handling depended on check order. BackupS3BlobBulkLoadRestoreNarrowFleet.toml provokes the condition, and its fleet size decides whether it asserts anything. Omitting extraStorageMachineCountPerDC left 5 machines against StorageTeamSize 3, where all C(5,3) machine teams touch src and no split can place the range -- the test demanded something no fix can deliver, failing about 1 seed in 29. At 2 it went vacuous: 0 of 17 seeds saw a placement failure. It is 1, the threshold. * BulkLoad: guard the split's write order, and correct the Unplaceable contract Comments and one assertion; no logic change. splitBulkLoadTask's two krmSetRange calls are correct only because children is built in ascending key order. krmSetRange reads oldValue at Snapshot::True on a plain Transaction, so each call is blind to the previous one's mutations and only their order makes the result right: ascending, the second call's clear() erases the boundary value the first parked there before rewriting it. Descending, the second call's trailing set(boundary, oldValue) lands last and republishes the parent over the second child's range -- exactly the state the tiling comment claims cannot exist. The loop reads as order-agnostic and nothing enforced it, so add the assertion and record why it is load-bearing. krmSetRangeCoalescing_ carries a REQUIREMENTS FOR CALLERS block for the same hazard class; plain krmSetRange has none. Outcome::Unplaceable claimed to be "neither transient nor a property of this attempt". It is sent from the generic BestTeamStuck path, which getTeamForBulkLoad reaches for three separately counted reasons -- src overlap, every team unhealthy, and no team with disk headroom -- and only the first is what narrowing addresses. Say that instead, including why splitting on the others is wasted rather than harmful: the children tile the parent and the split count is bounded by the manifest count. Also: splitBulkLoadTask's return contract named one of its three false paths, and omitted the one that is not a statement about the range at all (the parent is no longer ours). Dropped two comments describing the bool pair this PR removed, which no longer exists anywhere in the tree, and a note recording which test caught an earlier tiling bug. Signed-off-by: akankshamahajan15
…rd/remove-actor-compiler-20260827 Remove the Flow actor compiler and obsolete compiler tests
…rd/fdb-storage-coro-promotable-20260714 Reduce coroutine overhead on storage-server paths
…atch (apple#13886) The validate_restore audit phase ends only when its slowest task does, and nothing bounded that task: at 100M the slowest took 2535.86s of a 2536.0s phase, i.e. 100% of it. Per-task throughput improvements therefore could not move it -- an earlier attempt won 1.94x per task and made the phase 25% SLOWER, because that run's shards happened to be 1.87x fatter. It was optimising an average when the metric is a maximum. 1. Cap one audit task's range. scheduleAuditOnRange divided work by keyServers boundaries, one task per shard scanned sequentially by a single storage server, with no size cap anywhere in the dispatch path. Shards above AUDIT_TASK_MAX_BYTES are now subdivided via splitStorageMetrics, lazily one shard at a time so the first task dispatches immediately, falling back to the unsplit shard if the metrics read fails. minSplitBytes must not exceed half the target, because the storage server stops splitting once remaining.bytes < 2 * minSplitBytes: passing the target itself left 287 of 867 tasks stranded unsplit in the 128-256MB band. 2. Treat AUDIT_RESTORE_BATCH_BYTE_LIMIT as a ceiling and adapt under it. It previously used CLIENT_KNOBS->REPLY_BYTE_LIMIT (80KB), a per-RPC-reply cap reached long before AUDIT_RESTORE_BATCH_KEY_LIMIT at any realistic record size, so raising the key limit did nothing: a 1B validation spent 2h26m in ~3.6M two-round-trip iterations at ~5.5MB/s per server, against an AUDIT_STORAGE_RATE_PER_SERVER_MAX allowance ~9x higher. A fixed 4MB is no better -- it took transaction_too_old from 679 to 2515 on a 100M run -- because the constraint is a deadline, not a size, and overshooting it buys double work rather than throughput. The budget now halves on a retryable error and grows back additively above AUDIT_RESTORE_BATCH_BYTE_LIMIT_MIN. Also here: the two sides of a batch were read serially for two independent ranges at the same read version and are now issued together; retries use the existing Backoff helper instead of a flat 1.0s sleep; and per-attempt counters advanced before a progress-persist that can throw, so a batch failing there double-counted the counters the throughput analysis reads. 3. Raise SERVE_AUDIT_STORAGE_PARALLELISM to 4, since subdivided pieces landing on one server would otherwise serialize and the cap would buy nothing. Randomized 1..4 in simulation so both paths keep coverage. That exposes two things the knob being 1 had masked. The rate limiter was built per call site, multiplying an allowance whose knob is named AUDIT_STORAGE_RATE_PER_SERVER_MAX; it is now one per server. And auditStorageServerShardQ needs exclusive use of the server -- it owns shardAssignmentHistory, single-instance state -- so it takes a dedicated lock rather than relying on the permit count. Without it, the per-server retry in scheduleAuditStorageShardOnServer re-dispatches while the previous attempt is still running: at one permit the second request queued, at four it trips the "another audit is running" tripwire. Seen as SevError ExistStorageServerShardAuditExit on Sideband, InventoryTestSomeWrites and MoveKeysCycle, 3 hits across 20,000 runs. 4. Fix two pre-existing ways an ssshard audit wedges a storage server, both reachable without the change above. stopTrackShardAssignment() was reached only by plain statements, so a cancelled audit destroyed the coroutine frame without running any of them and stranded trackShardAssignmentMinVersion set, after which every later ssshard audit on that server bailed out for the life of the process; it now runs from a ScopeExit guard. And that bail-out sat between take() and the FlowLock::Releaser, leaking a permit from a lock shared by every audit type. 5. Two report storms. An Error-phase bulkload task is deliberately never erased, so scheduleBulkLoadTasks() re-reported it on every metadata rescan -- 11,313 events from 2 tasks in one 1B run, still climbing hours after the job finished. And the SS bulkdump loop retries up to 50 times by design while shards move underneath it, logging SevWarn per attempt -- 18,023 events in one run against 29,026 files. Intermediate retries move to SSBulkDumpRetry; terminal cases keep the original name and severity so existing greps match only real failures now. Per-batch validate_restore traces move behind ENABLE_AUDIT_VERBOSE_TRACE; at 1B they were ~18M unconditional SevInfo events. SSAuditRestoreRetryableError is suppressed with a running total at completion, since retrying is expected and one server logged 1,610 in a single audit. Measured at 100M before the fixes in 1 and 3: audit phase 42m16s -> 25m00s, 26% under the 33m47s pre-existing baseline; transaction_too_old 2515 -> 150; aggregate throughput 79 -> 130MB/s. These need retaking, since the minSplitBytes correction and the dispatch throttle both changed the task count. Split points become task ranges with the same consecutive-pairs idiom DDShardTracker uses; splitStorageMetrics() guarantees begin and end as its first and last points and NativeAPI rejects out-of-order replies, so no re-derivation is needed. nextAuditBatchBytes() is unit tested for the budget staying within [floor, ceiling], and boundAuditTaskRange() by a mock asserting minSplitBytes stays at or below half the target, since exceeding it silently leaves shards above the cap unsplit.
…rd/clang-tidy-safety-20260830 Enable clang-tidy checks for condition assignments, NUL strings, and vector growth
…rd/redwood-seek-cache-budget Give Redwood cache-hit test sufficient cache capacity
…ailable remote DC
MarkSh1
force-pushed
the
report-degraded-multi-region-status
branch
from
August 31, 2026 21:01
a2e8a4a to
37db9a9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
test