[fabwa]

Summary

storager handled one storageTask at a time, and each may need a network HEAD (m.net.Head) to fetch a header it does not already have. That call goes through the same get service as any object HEAD and may have to reach a remote peer. Since storager was the sole reader of its queue, a slow peer delayed indexing of every object queued behind it.

Fix

Resolve a group of up to storagerConcurrency headers concurrently into a positionally indexed slice, then hand them to batchWriter in the order they were queued. 16 is the bound split.Verifier already uses for the same kind of per-object HEAD. A group is one blocking take plus whatever is already queued, so a single task is still handled immediately.

Queue order is preserved deliberately: metabase.DB.PutBatch applies a batch in slice order within one transaction, so for a lock and a tombstone of the same object it decides which one is indexed and which is skipped as non-critical. Tasks arrive here in metadata chain order, and resolving them out of order would make that outcome depend on local header-fetch timing instead.

Scope

This does not help a peer that hangs rather than one that is slow: preserving order means head-of-line blocking, and Head runs on the service context with no deadline. The gain is that N slow-but-responsive peers cost one HEAD round trip rather than N.

Test plan

• TestMeta_storager_KeepsQueueOrder: tombstone queued first with a slow header, lock second with an immediate one, asserts the tombstone still wins the conflict. Verified to fail both on the previous worker-pool version and on a variant that fills the group in completion order.
• TestMeta_storager_BoundsConcurrency: verified to fail on master (serial), passes here.
• go test ./pkg/services/meta/... -race clean over 20 consecutive runs; go vet ./..., golangci-lint and go build ./... clean.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 32.87%. Comparing base (fd6e171) to head (d95f619).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4284      +/-   ##
==========================================
+ Coverage   32.78%   32.87%   +0.08%     
==========================================
  Files         673      673              
  Lines       41707    41711       +4     
==========================================
+ Hits        13673    13711      +38     
+ Misses      28034    28000      -34     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[carpawell]

While this PR should not break anything, cause only storage routines were added, i do not like it cause it was kinda a feature: every object goes one by one in a strict order, this is why the meta service was created. With this PR, the order of object savings to the local index may change now. HEADing in parallel and the common idea is ok to me, but i would have kept the original object order.

[fabwa]

Kept the worker pool rather than preserving order — my call, but say the word and I will switch.

The order already is not observable downstream: PutBatch is a single bbolt transaction over up to 100 objects keyed by address, so order within a batch is lost anyway; only objPutEvName reaches this queue, so there are no deletes whose position relative to a PUT could matter; a failed IsMineWithMeta/Head drops the object with no retry; and nothing persists a position to resume from (chainHeight is in-memory only).

I did write the order-preserving variant — fetch concurrently, hand off in arrival order — and did not push it because it reintroduces head-of-line blocking: one hung HEAD stalls everything after storagerConcurrency tasks, whereas the pool keeps working on the other 15. Head runs on the service context with no deadline, so that is exactly the case this PR is about. If you still prefer the original order, I will push that version instead.

[roman-khimov]

a single bbolt transaction over up to 100 objects keyed by address, so order within a batch is lost anyway

I don't think so. It's a slice and it's iterated one by one. So for a typical case of tombstone/lock conflict order matters a lot, one should succeed, the other should fail and the winner is determined by order defined by the order of transactions.

So while the problem is somewhat valid (certainly not critical at this point), the fix suggested is not correct.

[fabwa]

Confirmed, you're right. PutBatch iterates the slice inside one bbolt.Update, so each put sees the previous ones: for a lock and a tombstone of the same object, [TS, lock] indexes the TS and skips the lock, [lock, TS] does the reverse, and both calls return nil. My second argument was wrong the same way - ObjectPut carries no type filter and a tombstone is itself an object PUT, so PUT order is the delete order.

Reworked to keep it: a group of up to 16 headers is resolved concurrently into a positionally indexed slice and handed to batchWriter in queue order, same shape as verifyChildGroup. A group is one blocking take plus whatever is already queued.

Worth flagging that this no longer helps a peer that hangs rather than one that is slow, since preserving order means head-of-line blocking. What is left is the latency amortization. Title and description updated.