[ENH](wal3): pin fragment generations for conditional writes

[rescrv]

Description of changes

S3-backed conditional writes previously validated the committed log
against a moving target: a concurrent publish could open and close a
fragment generation between validation and append, forcing a retry.
Introduce FragmentPin so a caller can hold the next fragment generation
open while it reads the manifest and prepares its append, ensuring it
validates against the generation its append will join.

Add FragmentPin, a generation-scoped, must-use reservation:

• acquire_fragment_pin waits for the active generation to finish, then
  reserves bytes against the fragment size limit and returns a pin.
• Work submitted with a pin joins the pinned generation; dropping a pin
  releases its reservation so cancelled or invalidated requests make
  progress without an explicit cleanup call.
• take_work holds selected work until every pin submits or drops, and
  bumps pin_generation when a batch is selected for write.

Thread the pin through push_work, append_many_with_options, and
LogWriterTrait, and add acquire_fragment_pin to the trait and its
S3/replicated implementations. Supplying a pin disables transparent
contention retries because a pin cannot transfer to a recovered writer
generation.

In log-service, S3-backed conditional pushes now acquire a fragment pin
before validation and record the wait in a new
conditional_write_fragment_pin_wait_us histogram; replicated logs stay
serialized in Spanner and skip pinning. Factor the repeated writer
shutdown blocks in handle_errors_and_contention into shutdown_epoch.

Test plan

CI

Migration plan

No change to disk structures.

Observability plan

Counters + tracing

Documentation Changes

N/A

Co-authored-by: AI

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[github-actions]

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

• Can you think of any use case in which the code does not behave as intended? Have they been tested?
• Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
• If appropriate, are there adequate property based tests?
• If appropriate, are there adequate unit tests?
• Should any logging, debugging, tracing information be added or removed?
• Are error messages user-friendly?
• Have all documentation changes needed been made?
• Have all non-obvious changes been commented?

System Compatibility

• Are there any potential impacts on other parts of the system or backward compatibility?
• Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

• Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

[rescrv]

@claude review once

[rescrv]

@claude review once

[blacksmith-sh]

Found 1 test failure on Blacksmith runners:

Failure

Test	View Logs
worker/ work_queue::tests::integration::tests::test_k8s_integration_work_queue_fifo_and_filteri ng	View Logs

^{Need help on this PR? Tag @codesmith-bot with what you need.}

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[dbeglord]

Head of line blocking. If one transaction is very slow, and holds the generation open, it'll block all future generations right? Or can multiple generations be open at once?

[rescrv]

If one transaction is very slow you will get HOL on the generation. This is not a problem as there are a finite number of requests allowed in at once, and wal3 alredy does HOL blocking on the batch.

[graphite-app]

The can_reserve logic allows bypassing the batch size limit when the generation is empty. The condition occupied_bytes == 0 short-circuits the size check, allowing arbitrarily large reservations:

fn can_reserve(&self, byte_count: usize, batch_size_bytes: usize) -> bool {
    let occupied_bytes = self.occupied_bytes();
    occupied_bytes == 0 || occupied_bytes.saturating_add(byte_count) < batch_size_bytes
}

If occupied_bytes == 0, the function returns true regardless of whether byte_count >= batch_size_bytes. This allows a single reservation to exceed the configured fragment size limit, potentially creating fragments much larger than intended.

Fix: Remove the short-circuit and consistently enforce the size limit:

fn can_reserve(&self, byte_count: usize, batch_size_bytes: usize) -> bool {
    self.occupied_bytes().saturating_add(byte_count) <= batch_size_bytes
}

Or if allowing oversized first reservations is intentional (to prevent starvation), add:

fn can_reserve(&self, byte_count: usize, batch_size_bytes: usize) -> bool {
    let occupied_bytes = self.occupied_bytes();
    occupied_bytes == 0 && byte_count <= batch_size_bytes 
        || occupied_bytes.saturating_add(byte_count) <= batch_size_bytes
}

Suggested change

	fn can_reserve(&self, byte_count: usize, batch_size_bytes: usize) -> bool {
	let occupied_bytes = self.occupied_bytes();
	occupied_bytes == 0 || occupied_bytes.saturating_add(byte_count) < batch_size_bytes
	}
	fn can_reserve(&self, byte_count: usize, batch_size_bytes: usize) -> bool {
	let occupied_bytes = self.occupied_bytes();
	occupied_bytes == 0 && byte_count <= batch_size_bytes
	|| occupied_bytes > 0 && occupied_bytes.saturating_add(byte_count) <= batch_size_bytes
	}

Spotted by Graphite



Is this helpful? React 👍 or 👎 to let us know.

[rescrv]

I saw this and the alternative is a degradation from what we support today.