* util: retry transient errors, not just the ones containing "transport"
util.Retry only retried when the error string contained "transport", so a
plain "read: connection reset by peer" from S3 got zero retries. Classify
the error instead: net timeouts, connection resets, and the throttling and
overload replies S3 and gRPC return are all worth another attempt, while a
cancelled or expired context is not.
* filer sync: hold the sync offset behind a failed event
A sync job that returned an error was logged and forgotten, and the
watermark advanced past it anyway. The offset is the durable resume point,
so the event was never replayed: for filer.remote.sync that left the file
present locally, absent on the remote, with no RemoteEntry and nothing to
retry it.
Pin the watermark at the oldest failed event. Later events keep flowing,
but the persisted offset stays behind the failure, so a restart replays it.
On a read-only watched path the idle heartbeat keeps sync_offset fresh,
but a busy source filer still emits a MaxUnsyncedEvents marker after many
filtered events. The marker has a non-nil but empty EventNotification, so
the client routed it to the event path, where it advanced no real
watermark yet drove offsetFunc to republish the stale processed
watermark — regressing the gauge between heartbeats and spiking the
derived lag every time a filtered-event burst landed.
Route the empty marker through OnIdleHeartbeat like the idle heartbeat so
its fresh timestamp keeps the gauge current; it still advances the
in-stream resume cursor.
* perf(filer.sync): don't serialize descendants behind dir attribute updates
The MetadataProcessor treated every in-flight directory job as a subtree
barrier: any active dir job at /foo forced all file events under /foo to
wait, and because the admit loop runs on the single stream.Recv()
goroutine, a stalled descendant also stalled the whole gRPC stream. For
large directories this turned every attribute-only dir event (mtime /
xattr / chmod bumps) into a full-subtree pinch point.
Classify dir jobs as barrier (create / delete / rename) vs non-barrier
(filer_pb.IsUpdate on a directory — same parent and same name, i.e. an
in-place attribute update). Only barrier dirs block descendants and get
blocked by ancestor barrier dirs. Non-barrier dir updates still bump the
ancestor descendantCount, so an incoming barrier dir on an ancestor
still waits for them — preserving the "delete /a waits for in-flight
/a/b update" safety.
Tests cover the loosened cases and the preserved barriers:
non-barrier update doesn't block a file descendant, barrier create
still does, barrier delete still waits for in-flight descendants, and
a barrier ancestor still waits for a non-barrier descendant update.
* fix(filer.sync): serialize same-path barrier dir jobs against concurrent ops
Review (Gemini) flagged that pathConflicts had latent same-path gaps
that predated this PR but deserve fixing alongside the dir-conflict
loosening: two barrier dir jobs at the same path could run concurrently
(e.g. create /a and delete /a), and a file job at the same path as an
in-flight barrier dir wasn't blocked either.
Tighten pathConflicts so that:
- an active barrier dir at p blocks every incoming job at p (file,
barrier dir, or non-barrier attribute update) — same-path promotions,
renames, and delete/create collisions must serialize;
- an active file at p blocks incoming files and barrier dirs at p;
- non-barrier dir updates at the same path still overlap with each
other (attribute bumps are last-writer-wins, intentional).
TestDirVsDirConflict and TestFileUnderActiveDirConflict flip their
"same path does not conflict" assertions to match. New
TestSamePathBarrierSerialization covers all five same-path cases
explicitly.
* fix(filer.sync): serialize incoming barrier dir against same-path non-barrier update
Bug introduced by the previous same-path tightening commit and caught
in review (CodeRabbit, critical): a kindNonBarrierDir at /dir1 was not
indexed at its own path, so a later kindBarrierDir at /dir1 saw neither
activeBarrierDirPaths["/dir1"] nor descendantCount["/dir1"] (the latter
only counts strict descendants) and was admitted concurrently with the
in-flight attribute update. That violated the "barrier at p serializes
all work at p" rule.
Track non-barrier dir jobs in a new activeNonBarrierDirPaths map and
check it only from the incoming-barrier-dir branch of pathConflicts.
The map is deliberately invisible to the ancestor check, so non-barrier
updates still don't serialize file descendants — the loosening this PR
is about stays intact.
Regression test added in TestSamePathBarrierSerialization covers both
the admission conflict and the index cleanup on job completion.
* filer.sync: replace O(n) conflict check with O(depth) index lookups
The MetadataProcessor.conflictsWith() scanned all active jobs linearly
for every new event dispatch. At high concurrency (256-1024), this O(n)
scan under the activeJobsLock became a bottleneck that throttled the
event dispatch pipeline, negating the benefit of higher -concurrency
values.
Replace the linear scan with three index maps:
- activeFilePaths: O(1) exact file path lookup
- activeDirPaths: O(1) directory path lookup per ancestor
- descendantCount: O(1) check for active jobs under a directory
Conflict check is now O(depth) where depth is the path depth (typically
3-6 levels), constant regardless of active job count. Benchmark confirms
~81ns per check whether there are 32 or 1024 active jobs.
Also replace the O(n) watermark scan with minActiveTs tracking so
non-oldest job completions are O(1).
Ref: #8771
* filer.sync: replace O(n) watermark rescan with min-heap lazy deletion
Address review feedback:
- Replace minActiveTs O(n) rescan with a tsMinHeap using lazy deletion.
Each TsNs is pushed once and popped once, giving O(log n) amortized
watermark tracking regardless of completion order.
- Fix benchmark to consume conflictsWith result via package-level sink
variable to prevent compiler elision.
The watermark advancement semantics (conservative, sets to completing
job's TsNs) are unchanged from the original code. This is intentionally
safe for idempotent replay on restart.
Chris Lu