Skip to content

benchmarks: durdir benchmark that evaluates the efficacy and performance of DurDir - #907

Merged
Max Smythe (maxsmythe) merged 10 commits into
agent-substrate:mainfrom
sairajp-rewind:benchmarks/durdir-673
Aug 19, 2026
Merged

benchmarks: durdir benchmark that evaluates the efficacy and performance of DurDir#907
Max Smythe (maxsmythe) merged 10 commits into
agent-substrate:mainfrom
sairajp-rewind:benchmarks/durdir-673

Conversation

@sairajp-rewind

Copy link
Copy Markdown
Collaborator

Benchmark DurDir

Fixes #673

Adds a DurDirUser load-generation workload that exercises the DurableDir
suspend/resume loop end to end, verifies every served byte against a SHA-256
digest, and emits separable latency percentiles for each step.

The loop

per VU, first iteration:  create -> resume -> WriteDisk -> ReadDisk+verify
steady state:             suspend -> resume -> ReadDisk+verify (cold)
                                  -> ReadDisk+verify (warm)
                                  -> WriteDisk (overwrite, TRUNCATE)

Under onCommit: Data the container cold-boots from the OCI image and process
memory is discarded, so a matching digest after resume can only have come from
the restored DurableDir. That is the durability assertion.

Results

All six scenarios ran on GKE/gvisor at 1 VU for 1m each: Data vs Full
snapshot scope, explicit vs implicit resume, and a 5/10/64 MiB size sweep.
Zero failures across every run, so every served byte matched its digest in
all six.

Snapshot growth over repeated overwrites: SuspendActor latency stayed flat
across consecutive overwrite-and-suspend cycles on the same DurableDir volume.
The loop overwrites with WRITE_MODE_TRUNCATE, so the file is exactly X bytes
after every write and the captured directory contents are the same size every
cycle. Nothing accumulates across suspends. That is the growth question the
issue asks about.

Latency percentiles per step are in the run artifacts.

Change surface

  • glutton: WriteDisk returns size + sha256; new ReadDisk with a
    READ_MODE_DIGEST_ONLY mode for measuring restore cost without paying wire
    transfer; disk RPCs exposed over HTTP mode.
  • manifests: two new ActorTemplates, glutton-durdir-{data,full}, with a
    durableDir volume and onPause: Full / onCommit: {Data,Full}.
  • boomer: shared actor-lifecycle plumbing extracted from the ping task, then
    a DurDirUser task on top of it; a general resume_mode knob.
  • harnesses: --workload selects the task at deploy time; durdir.py stub,
    typed dynconfig flags, six nightly scenarios.

The two boomer commits above are incremental extractions made as the second
workload landed. The final package layout for internal/benchmarking/boomer
lands as a follow-up PR.

Testing

  • go build ./..., go test -race ./cmd/benchmarking/... ./internal/benchmarking/...
    clean, no race warnings.
  • hack/verify-all.sh: all nine checks pass. Python protos regenerated, tree
    clean.
  • Both ActorTemplates reach Ready; golden snapshots confirmed in the bucket.
  • Manual durability proof: 200 MiB payload written, suspended, resumed, and
    re-read with matching sha256 across every read. Process memory discarded and
    container cold-booted in between.
  • Scale validation: DurableDir persistence verified up to 1 GiB with 0
    failures.
  • Regression gate: glutton_baseline_5_users ran 919 requests with 0
    failures and 7 ms ping latency post-rebase. No regression on the existing
    benchmark.

Deliberately out of scope

  • Image size over time: ActorSnapshot has no size field, so there is
    nothing for a client to read. The issue permits deferring this. SuspendActor
    latency is the available proxy; atelet.snapshot.size is the server-side one.
  • boomer package restructure: landing as a follow-up so this PR's files stay
    reviewable in place.
  • RAM-backed variant: glutton already has WriteRAM; small follow-up.

@maxsmythe Max Smythe (maxsmythe) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this.

Left some comments.

Also, we should avoid sending 4k line PRs where possible -- they take multiple hours to review closely.

Can we avoid refactoring anything under internal/benchmarking/boomer for now? I don't think the abstractions are quite right and the refactoring is a large portion of the delta. We can implement ~3-4 tests and then figure out what the best refactor is.

help="Size in bytes written to /var/lib/glutton/bench-data (default 8 MiB)",
include_in_web_ui=True,
)
parser.add_argument(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this applies to any workload, it should probably live in its own Python file

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split out. --resume-mode now lives in common/resume_mode.py

# if those disagree the stats rows describe traffic that was never sent.
# The value must name a stub in locust/tests/, a --workload case in
# cmd/benchmarking/boomer-glutton/main.go, and an entry in runner.py's
# BOOMER_WORKLOADS dict.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: look to reduce # of sources of truth

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

Comment thread benchmarking/locust/runner.py Outdated
return args


def boomer_workload(test_file: str) -> str | None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to test for Python workloads... I suspect we will no longer be adding those, so it will be a static list. A cleanup work item will be to remove those tests.

This will also remove one place workload lists need to be kept in alignment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went with a single static map from stub filename to user class:

BOOMER_USER_CLASSES = {"glutton.py": "GluttonUser", "durdir.py": "DurDirUser"}

needs_boomer is membership in it and the same entry supplies --user-class, so the filename and the class name stay in one place rather than two.
Kept the check on the boomer side rather than the Python side because runner.py needs the class name string either way

Comment thread benchmarking/README.md Outdated
python3 runner.py -f tests/durdir.py -t 1m -u 1 --name durdir_run --dest /tmp/bench \
--durdir-template glutton-durdir-data --resume-mode explicit --durdir-file-size-bytes 8388608
```
* **Interactive Web UI**: The stack runs one workload per deployment, chosen at deploy time:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think "workload" is correct... that is the name given to the ActorTemplate and workers.

One "test type"? "Virtual user type"?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UserClass?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment thread benchmarking/README.md Outdated
* `glutton-durdir-data` (default): Attaches a durable data directory under `/var/lib/glutton` without memory snapshot restore.
* `glutton-durdir-full`: Attaches a durable data directory and performs a full memory snapshot restore.

#### File-Size Sweeps

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to document that we can run multiple tests by adding multiple tests. Also, I think setting the read mode to digest would be orthogonal to "sweep".

I'd remove this section.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

cfg *Config
actorName string
hostHeader string
userClass string

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea, though we will probably need some design/refactoring to make this truly generic across all users. We'll tackle that more on a later test.


// resume brings the actor up before the next request.
//
// The first resume is always explicit, whatever the mode. It is the only

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is far too much context for a comment. Documents the code and likely to rot quickly.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

user *gluttonUser
expectedDigest string
fileSize int64
readMode gluttonpb.ReadMode

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things that are stored as dynamic config should not be persisted to the class, they should be read from the dynamic config, so behavior can change dynamically (might need to have some read atomicity to avoid breaking behavior due to time aliasing)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, including the aliasing half.

slog.Info("configured durdir file size", slog.Int64("bytes", fileSize), slog.String("template", tmpl))

if readMode == gluttonpb.ReadMode_READ_MODE_DATA && fileSize > 64*1024*1024 {
slog.Warn("durdir_file_size_bytes > 64MB with durdir_read_mode=data may hit router or Envoy timeouts; set durdir_read_mode=digest",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to warn about this... if we are hitting timeouts that would be the point of the benchmark, to find out.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

# its own per-worker diagnostics at /metrics on :8001 (aggregate stats
# flow through the master via boomer.RecordSuccess).
#
# ${BENCHMARK_WORKLOAD} is substituted by benchmarking/locust/deploy.sh

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment describes too many internal details

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

@BenTheElder

Copy link
Copy Markdown
Collaborator

let's squash this on merge (and more PRs, generally, unless there's really useful commit history)

@sairajp-rewind
Sairaj Pokale (sairajp-rewind) force-pushed the benchmarks/durdir-673 branch 2 times, most recently from b7e91e5 to 98916bc Compare August 13, 2026 23:33

@maxsmythe Max Smythe (maxsmythe) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Couple of comments

_FLAGS = ("--trace-probability", "--min-wait-time", "--max-wait-time")
# Boomer-tunable flags and their types. CLI form ("--foo-bar") is converted
# to the attribute / JSON-key form ("foo_bar") by _attr().
_FLAGS = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a thing for this review, but probably the correct way to do this is to add each flag to a unified registry upon import.

Again, not a thing for this PR but a cleanup for later to help remove duplicate sources of truth.

Comment thread benchmarking/locust/deploy.sh Outdated
echo " -h|--help Show this help message"
echo " --deploy Deploy the locust workers"
echo " --delete Delete the locust workers"
echo " --user-class NAME User class to deploy: GluttonUser | DurDirUser (default: GluttonUser)"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's not use case-sensitive arguments (just do lowercase). Also, we can name the class after the file.

Let's assume a convention of /tests/{user_class}.py (e.g. GluttonUser becomes glutton).

That also removes the need to do mapping and argument correctness check is just looking for the existence of the correct file.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adopted. --user-class is lowercase and case-insensitive, the file is
tests/<value>.py, and the class is <Value>User — so glutton ->
GluttonUser, durdir -> DurdirUser. Renamed the Python class to match the
convention rather than renaming the file.

deploy.sh validation is now just a file-existence check, BENCHMARK_USER_FILE
is gone from the manifest, and runner.py's filename->class map is gone
entirely — the flag value is the test file's stem.

Comment thread benchmarking/locust/runner.py Outdated


# Maps locust test filename to boomer's --user-class flag.
BOOMER_USER_CLASSES: dict[str, str] = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adopting the filename == user + .py convention also removes the need for this code.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed

Comment thread benchmarking/locust/runner.py Outdated
"""Return True if the test file is the glutton stub; the real GluttonUser
implementation lives in the boomer-glutton binary."""
return os.path.basename(test_file) == "glutton.py"
"""Return True if the test file's User class runs on boomer."""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per previous review, let's flip this to test whether the user class needs Python. membership of that set should be static, reducing brittleness.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flipped. PYTHON_TESTS is the static set and anything not in it routes to
boomer.

Comment thread benchmarking/README.md Outdated
### DurDir Benchmark

The DurDir benchmark evaluates actor suspend/resume performance, disk persistence overhead,
and state restoration latency when durable actor directories (`/var/lib/glutton`) are attached.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove reference to "var/lib/glutton" -- too much implementation detail.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment thread cmd/benchmarking/glutton/main.go Outdated
// docs/dev/best-practices/tracing.md: extract incoming context,
// then name the span after the operation in each handler.
handler = otelhttp.NewHandler(mux, "/")
handler = otelhttp.NewHandler(newMux(svc), "glutton-http")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we renaming "/" "glutton-http"?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not intentional, leftover from when the mux had a single route. Reverted to
"/", which is what docs/dev/best-practices/tracing.md prescribes: the root
mux gets "/" and the operation name comes from the per-handler span. That's
already what protoRoute does, so Ping/WriteDisk/ReadDisk show up as
children.

Comment thread cmd/benchmarking/glutton/main.go Outdated
if _, err := f.Seek(0, io.SeekStart); err != nil {
return nil, status.Errorf(codes.Internal, "seek %s: %v", path, err)
}
h := sha256.New()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we want to always pair reads with writes.

It is necessary if we want the full SHA for a file that was only partially overwritten, otherwise, can we calculate SHA as data is written to disk?

that way people can benchmark pure writes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good Catch. Fixed by branching on the write mode. Under TRUNCATE the file is exactly N
bytes by construction, so the digest streams through an io.MultiWriter during
the write and there is no read-back at all. OVERWRITE is the only mode that
can leave bytes past N from a larger earlier write, so that branch keeps the
re-hash. Both existing tests pin the two paths, unchanged.

resumeBoots []bool
}

func (f *fakeControlClient) CreateAtespace(ctx context.Context, in *ateapipb.CreateAtespaceRequest, opts ...grpc.CallOption) (*ateapipb.Atespace, error) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there no other place where API server has been faked, where we can use that? Can we centralize this so only one fake needs to be created?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went looking — five packages already do this, and every one is test-local, so
none of them is importable:

  • cmd/atecontroller/internal/controllers/actortemplate_controller_test.go:98
  • cmd/atenet/internal/router/health_test.go:42
  • cmd/atenet/internal/router/ingress/ingress_test.go:40
  • cmd/atenet/internal/router/ingress/resumer_test.go:33
  • cmd/atenet/internal/router/egress/egress_test.go:198

(internal/testenv is envtest running a real Kubernetes apiserver, so it's a
different thing.)

So the duplication is real, but centralizing means a new non-test package plus
migrating five call sites across atecontroller and atenet. Happy to take it as a
follow-up.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's file it as an issue.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added: #1044

// The data slice is the source of truth: both routes report len(data) and
// sha256(data), and /readdisk serves data as payload.
// Each override field makes the actor lie about exactly one property.
type diskServer struct {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should just create a mock glutton server as part of glutton, vs. relying on each test to make its own mock.

bonus... can probably do this by just creating shims in actual server code for anything that has a side effect (like writing to disk), then use dependency injection to differentiate between a real and fake server.

@sairajp-rewind Sairaj Pokale (sairajp-rewind) Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

main.go:18 is package main, so the service isn't importable, a shared mock
means moving those 698 lines into a real package first.

The in-memory store would also have to model on-disk residue (main.go:457, the
OVERWRITE branch) or it goes green while the server is wrong, the bug from
the thread above. glutton's own tests use t.TempDir() for that reason.

Narrower point: the override fields at fixture_test.go:99-106 exist to make the
actor lie so the client's verification path can be proven to catch it, so some
lying variant stays here regardless.

Follow-up alongside the apiserver fake, one PR or two?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets move the fake to internal/benchmarking/glutton/fake

We should factor glutton's core logic from main.go into internal/benchmarking/glutton/..., but that can be done later.

Override fields on fakes can be public, no need to localize that code (indeed, the point of centralized fakes is to make it easy to override behavior in multiple places).

Also, to be sure, there is no need to write tests for the test fixture, so this should minimally impact line count.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to internal/benchmarking/glutton/fake; diskServerfake.Server with the
override fields exported. No tests for the fixture.

One deliberate duplication: the fake declares its own WriteDiskRoute/ReadDiskRoute
instead of importing the client's (durdir.go:54-55), so it depends on nothing. Both
collapse onto one definition when glutton's core moves into internal/benchmarking/glutton/....

Comment thread internal/proto/glutton/glutton.proto Outdated
WRITE_MODE_OVERWRITE = 1;
}

// ReadMode selects how much of the file ReadDisk sends back. Digest-only

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shorten this doc string, assume a user knows why they would want functionality, they just need to know what a thing does.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@sairajp-rewind
Sairaj Pokale (sairajp-rewind) force-pushed the benchmarks/durdir-673 branch 2 times, most recently from a17cb0f to 8f3d41d Compare August 15, 2026 01:42

@maxsmythe Max Smythe (maxsmythe) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting close, a few more comments to sort out, then the tests are left for reviewing.

Comment thread benchmarking/README.md Outdated
`--dest`. This is what the nightly automation uses.

```bash
python3 runner.py -f tests/<name>.py -t 1m -u 1 --name <run-name> --dest /tmp/bench

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name => user class

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment thread cmd/benchmarking/boomer-glutton/main.go Outdated
)

switch class {
case "glutton":

@maxsmythe Max Smythe (maxsmythe) Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General pattern for things like this (unknown set of extensible dependencies) is to declare the linkage next to the code that implements it and add it to a registry. See the type code for KRM objects:

func init() {
SchemeBuilder.Register(&ActorTemplate{}, &ActorTemplateList{})
}

in our case, something like:

registry.go

type Entry struct {
   name string
   locustFile string
   userClass string
   initFunc func(Config) (func(), func())
}

var reg map[string]entry

func Add(entry Entry) {
   reg[name] := entry{locustFile: locustFile, userClass: userClass, initFunc: initFunc}
}

func Init(name string, cfg Config) (func(), Func()) {
   return reg[name].initFunc(cfg)
}

// etc...

userclass.go

import registry // if not in same package (which it normally wouldn't be)

entry := registry.Entry{
    name: "something",
   locustFile: "something.py",
   userClass: "SomethingUser",
   initFunc: initFunc,
}

registry.Add(entry)

etc.

Registry class should live inside internal/benchmarking directory tree.

This will make it so newer entries do not need to update main.go

I realize that this will require a (hopefully minor) update to the glutton user class file.

Also, the term "register" is now overloaded... original usage of the term should probably be updated to "initBoomer" etc. anyway... "register" is a misnomer.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, as internal/benchmarking/boomer/userclass/. Entry{Name, LocustFile, UserClass, Init}, with Add/Lookup/Names; Add panics on a duplicate name.
Both classes register from func init() next to their implementation - lifecycle.go:62 and durdir.go:62 - matching the SchemeBuilder.Register pattern you linked. main.go no longer has a per-class branch; adding a class touches only its own file.
Also renamed the overloaded term: RegisterPing is now initPing.

Comment thread cmd/benchmarking/glutton/main.go Outdated
mux.HandleFunc("/readyz", func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
})
mux := readyzMux()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this mux is only used in the gRPC case and should only be defined there.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split into two constructors, each called at its use site. readyzMux() is just the probe, and gRPC mode passes it to splitGRPC as the non-gRPC handler (main.go:135). HTTP mode calls newMux(svc) (:184), which layers the three proto routes on top of the same probe. Nothing is built for a mode that doesn't use it.

Comment thread cmd/benchmarking/glutton/main.go Outdated
// OVERWRITE leaves any bytes past size from a larger earlier write, so the
// streamed digest covers only a prefix. Re-hash from disk to get the real one.
if req.GetWriteMode() == glutton.WriteMode_WRITE_MODE_OVERWRITE {
if _, err := f.Seek(0, io.SeekStart); err != nil {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it not possible to continue reading from the pre-existing cursor location until the end of the file, continuing to compute the SHA?

We should avoid double-reading. I believe I had suggested this in a previous comment.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, no re-read of the prefix in either mode. The digest streams through io.MultiWriter(f, h) as the bytes are generated (main.go:451). TRUNCATE is complete at that point. OVERWRITE is the only mode that can leave a tail from a larger earlier write, and the cursor is already sitting at size, so it just continues io.Copy(h, f) to EOF and folds that into the same digest. The response also returns the real on-disk size rather than the requested one, so it lines up with ReadDisk.

func (u *durDirUser) bootstrap(ctx context.Context, dynCfg dynconfig.Config) error {
fileSize, readMode := u.params(dynCfg)

// First resume: always explicit to warm up actor from golden snapshot

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure explicit resume is necessary on first boot as long as actor exists?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bootstrap now uses the configured ResumeMode like every other iteration, so implicit mode issues no ResumeActor at all; the forced-explicit special case and its test are deleted.

@maxsmythe Max Smythe (maxsmythe) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after two nits.

Thank you for the PR!

Comment thread cmd/benchmarking/glutton/main.go Outdated
return nil, status.Errorf(codes.Internal, "write %s: %v", path, err)
}

// OVERWRITE has no O_TRUNC, so bytes past size can survive a larger earlier

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

"bytes past size can survive a larger earlier write" -> "bytes from a larger, earlier write will persist"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

Comment thread benchmarking/README.md

### Headless

`runner.py` runs a test without the web UI, writing CSVs, logs and traces to

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: clarify that this is a command that is invoked by automation as a job on workload cluster. It is not meant to be run on a local machine to run tests in a headless mode.

Parallel documentation with deploy.sh makes it seem like the two approaches have equivalent UX.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

DeleteActor requires SUSPENDED or CRASHED; the bootstrap-failure path
deleted a running actor, leaking it and eventually exhausting workers.
Observed in the microvm sweep as 10× not in a deletable status
(status: STATUS_RUNNING) followed by no free workers available.
Move the httptest-backed fake glutton server out of boomer/glutton into
internal/benchmarking/glutton/fake so it can be shared by other benchmark tests.

The WriteDiskRoute and ReadDiskRoute constants are duplicated in the fake
package deliberately so it has no dependency on the boomer client; both sides
will collapse onto a single definition when glutton's core is factored out.
@maxsmythe

Copy link
Copy Markdown
Collaborator

LGTM, thank you for all the work!

@maxsmythe
Max Smythe (maxsmythe) merged commit ba45517 into agent-substrate:main Aug 19, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Benchmark DurDir

3 participants