aimdb-sync: drop `try_set`, finish the doc pass
@harunugurlu is already working on this.
Since Aug 15, 2026.
- Dominant language
- Rust
- Stars
- 100
- Forks
- 18
- Avg merge
- 19h 47m
- Merged PRs (30d)
- 33
Description
Revised 2026-09-16 against
f1fea6c, after the aimdb-sync rework
(#226–#239) landed under this issue. Line references refreshed throughout.
Decisions taken since the last revision:
- §2 (pre-resolve the producer) is dropped. It would reverse the
lazy-producer contract #235 established. Its one leftover, a doc item, moved
to §3.- §1 must merge before
aimdb-sync0.6.0 is published.SetTimeoutand
try_set_valuenever shipped, so removing them is free until then.- No new measurement. The figures in
fork.rsgo the way of the other
claims (§4).# Panicsstays per method, and now coversattach()too (§3).New since the last revision:
tests/fork_safety_test.rsusestry_set; the
README also documentsset_with_timeoutand severalDbErrorvariants that no
longer exist.
Context
#208 removed the internal channel bridge in aimdb-sync: blocking calls now go
straight to the runtime through the block_on seam, and SyncProducer holds
a Weak<Runtime> plus the record key, resolving the record on every set().
Two things were deliberately left out of that PR as follow-up work: pre-resolving
the producer, and dropping try_set now that no buffer can refuse a value. The
first has since been dropped (§2). The documentation items the review raised are
collected here too — along with the crate README, which the review never opened
and which still describes the deleted bridge.
The sections are commit-sized and listed in the order they should land: §1
deletes surface whose wording §3 would otherwise have to fix, and §4 rewrites the
one file no one has looked at since the bridge existed. §5 is optional and must
not hold the branch. Line references are as of f1fea6c.
This issue is self-contained. Every claim it touches is removed, not
remeasured — there is no benchmark work queued behind it and none is required to
close it. See §4.
1. Drop try_set
AimDb::produce is infallible on the push (aimdb-core/src/builder.rs:988-990)
and every buffer today overwrites rather than refusing, so set() and
try_set() are the same operation with different error mapping.
Timing: merge before aimdb-sync 0.6.0 is published (crates.io is at 0.5.0).
Of the three things removed, only try_set shipped in 0.5.0 — it returned
DbResult then. SyncError::SetTimeout and try_set_value are new in the
unreleased 0.6.0, so until it is published, removing them breaks no one.
- Remove
SyncProducer::try_set(producer.rs:147-188) andtry_set_value
(producer.rs:243-246). - Remove
SyncError::SetTimeout(error.rs:36-38), itskind()arm
(error.rs:76) and its test assertion (error.rs:121). - Drop the
try_setexample fromSyncProducer's type docs (producer.rs:31-35),
the bullets atlib.rs:16,lib.rs:146andlib.rs:160-161, and the feature
line inexamples/sync-api-demo/src/main.rs:171. - Tests:
test_runtime_shutdown_error_non_blocking
(tests/integration_test.rs:268) loses its producer half (:277);
integration_test.rs:185switches toset;
try_set_value_is_non_blocking_and_produces(tests/settable_integration.rs:67)
goes withtry_set_value;tests/fork_safety_test.rsdropsrefused_try
(:55-58, used at:70, and "the two publishes" at:60). - CHANGELOG: record the removal of
try_setunder[Unreleased] → Changed (breaking). Record nothing for the other two — instead droptry_set_value
from the unreleasedAddedentry (CHANGELOG.md:138), andtry_set()from the
fork-check entry (CHANGELOG.md:87). fork.rs:18justifiespthread_atforkwith atry_settiming. §4 rewrites
that paragraph.aimdb-sync/README.md:146-148,274,287documentstry_setagainst the
pre-#208DbError::SetTimeout. Leave it to §4 — the whole file needs
rewriting, not three line fixes.
This also removes an incidental cost: try_set calls db.producer(&self.key)?
per call (producer.rs:183), which takes impl Into and so clones the key every
time.
TryProduceError::Full stays in core; only the facade wrapper goes. No
buffer anywhere in the tree overrides WriteHandle::try_push
(aimdb-core/src/buffer/traits.rs:170) — the only overrides are the two test
doubles in that file's own mod tests (:444, :455) — so Full is
unreachable at every layer today, not just this one. The core variants stay
because they are the extension point a bounded, non-overwriting buffer would be
built against, and their docs should be corrected to say so
(aimdb-core/src/typed_api.rs:163-180 still describes backpressure). The facade
wrapper is the opposite trade: it duplicates set() for as long as no such
buffer exists, and re-adding it if one lands is cheap.
2. Pre-resolve the producer — dropped
Decided against on 2026-09-16. Since this issue was filed, #235 made lazy
creation the crate's contract:
handle.producer()touches neither the database nor the runtime thread
(handle.rs:344-357).test_error_propagation(tests/integration_test.rs:407) pins that an
unregistered key yields a producer andset()reports it.- The unreleased CHANGELOG records that a forked child's refusal lands on the
firstset()(CHANGELOG.md:85-87).
Pre-resolving would reverse all three, for less than it used to buy: set() must
still go through Runtime::db() for the fork and shutdown checks, so only the
keyed lookup would go. The one leftover is documentation — producer() cannot
fail and does not say so — and that is now in §3.
3. Documentation left over from the #208 review
set() describes behavior it no longer has
aimdb-sync/src/producer.rs:112-121 still promises blocking, guaranteed eventual
delivery, and a buffer-full error. None of the three can happen. set() checks
the runtime (RuntimeShutdown, ForkedChild), looks the record up
(Db(RecordKeyNotFound), Db(TypeMismatch)), and pushes, which cannot fail.
Same wording survives at producer.rs:28 and producer.rs:138.
The matching bullet in aimdb-sync/src/lib.rs:15 ("Blocking send, waits if
channel is full") is the last mention of the bridge's channel in the crate's
docs. lib.rs:157-159 needs the same fix: the error half is right, the blocking
half is not.
producer() cannot fail, and does not say so
AimDbHandle::producer() (handle.rs:320-343) returns SyncResult but always
succeeds — see §2. Its rustdoc should say so and name where key, type and fork
errors surface instead: the first set(). Today only SyncProducer::check()'s
doc hints at it.
# Panics on the blocking entry points
get and get_with_timeout call Handle::block_on (consumer.rs:125, :170);
get_latest and get_latest_with_timeout reach it through get_catch_up. It
panics with "Cannot start a runtime from within a runtime" if the calling
thread is already driving tasks. Both attach() methods have the same panic:
they wait for startup with blocking_recv (handle.rs:177), which panics inside
a runtime too (AimDbBuilderSyncExt::attach, handle.rs:53;
AimDbSyncExt::attach, handle.rs:88).
The crate now states this once, at crate level (lib.rs:204-207). Give each of
those six methods its own # Panics section as well, consistent with that note.
set() no longer enters the runtime, so producers need none.
Whether get() should also try reader.try_recv() first and enter block_on
only on BufferEmpty is a separate question and explicitly not part of this
issue; the docs are correct under either outcome, since such a fast path narrows
when the panic fires without removing it.
Consumer error docs
get_latest_with_timeout's# Errors(consumer.rs:280-283) omits
SyncError::Db, whichget_latestlists (consumer.rs:231-235).- Neither method says that
BufferLaggedis skipped rather than returned —
which is the behavior #208 introduced. get_latest_with_timeout's# Arguments(consumer.rs:278) says the timeout
bounds the first value; add that the drain after it is unbounded. In practice
the drain outruns a hot producer (a ring read is cheaper than a keyed produce),
so this is a doc fix, not a code fix.
CHANGELOG: lag now reaches callers
Dropping the forwarder changed observable behavior, not just the API: lag now
surfaces to callers of get() / get_with_timeout() / try_get() as
SyncError::Db(DbError::BufferLagged { .. }), where the old forwarding task
swallowed it. The rustdoc says so (consumer.rs:99-100); the CHANGELOG's #200
entry (CHANGELOG.md:155) does not. Add one bullet there, same unreleased 0.6.0
section.
Small stuff
- Typos: "occuring" (
consumer.rs:233,consumer.rs:319,
tests/integration_test.rs:361), "occured" (consumer.rs:235,
consumer.rs:348), "succesfully" (consumer.rs:263). consumer.rs:169: theasync { ... .await }wrapper is redundant —
block_on(tokio::time::timeout(...))works directly.handle.rs:370uses# Errors (wrapped in SyncError::Db); the rest of the
crate uses a plain# Errors.
4. Rewrite aimdb-sync/README.md and clear the last performance claims
The #208 review never touched this file, and neither did the rework since.
It still documents the channel bridge as the crate's architecture, and API that
no longer compiles:
- Lines 26-30 — a
Channel Bridge (tokio::sync::mpsc + std::sync::mpsc)box
in the architecture diagram, the defect the reviewer caught and had fixed in
lib.rs. - Lines 136, 387 —
set_with_timeout, removed in #200 (CHANGELOG.md:156). - Lines 146-148 —
try_set("Channel full or error"); goes with §1. - Lines 317-328 — a
Channel Capacitysection forproducer_with_capacity/
consumer_with_capacity, removed in #200 (CHANGELOG.md:157). - Lines 274-290 —
DbError::SetTimeout,DbError::RuntimeShutdown,
DbError::RecordNotFound,DbError::AttachFailed. None exist; the facade's
own failures areSyncErrorvariants. - Lines 450-452 — Optimization Tips: "Use
try_*methods" (wrong for
producers once §1 lands), "Channel Capacity", andruntime_threads, which is
not a setting.
The performance claims are the part that matters
Four claims are left in the crate. All four go in this pass:
README.md:444— "Channel crossing adds ~1-10μs latency".README.md:446— "Memory: One tokio::mpsc channel per producer, one
std::mpsc channel per consumer".src/lib.rs:135— "Latency: Excellent for <50ms target, not suitable
for hard low-latency requirements". It is the only bullet under
## Performance(lib.rs:133), so the heading goes with it.src/fork.rs:18-21— "Measured:try_setis 121 ns and
std::process::id()is 321 ns". Added by #230, after this issue was filed.
It cites no benchmark, and it namestry_set. Keep the reasoning — a relaxed
atomic load per publish instead of a pid read — and drop the figures.
The first two describe a bridge that no longer exists; the other two cite no
benchmark. This is the same family as the ~100–500μs per operation claim #208
removed from the crate docs, and they go the same way: delete them rather than
correct them.
There is no number to replace them with — aimdb-bench has no aimdb-sync
target — and none should be estimated. Building one is out of scope here and
is not queued behind this issue; the crate is expected to ship claim-free.
If a figure ever returns to these docs it must cite the benchmark that produced
it. Do not carry a placeholder forward.
Rewrite the README against the current design: the block_on seam, set()
pushing straight into the record's buffer, SyncConsumer holding a Reader
and blocking only when a read has to wait, the &mut self / no-Clone consumer
signature, SyncError, and the shutdown contract the CHANGELOG already records.
The Optimization Tips section should survive only where its advice is still true.
5. Optional: cover the mid-drain lag path
drain_remaining (consumer.rs:343) skips BufferLagged and keeps draining
(:347) — the fix for the review finding that a lag mid-drain used to end the
loop early and return a value that wasn't the latest. It still has no test: the
two get_latest tests (tests/integration_test.rs:341, :383) produce every
value before reading, so a lag can only be hit in get_catch_up. Triggering it
needs a producer overrunning the reader between the first read and the drain,
which is hard to make deterministic — worth doing only if it can be done without
a racy test. Do not hold the branch for it.
Acceptance criteria
try_set,try_set_valueandSyncError::SetTimeoutare gone, merged before
aimdb-sync0.6.0 is published. The CHANGELOG records the removal of
try_setand no longer mentions the other two.AimDbHandle::producer()'s rustdoc says creation cannot fail and that key,
type and fork errors surface onset().test_error_propagationand
test_runtime_shutdown_errorpass unchanged.- No producer doc describes blocking, a channel, or a buffer-full error.
get,get_with_timeout,get_latest,get_latest_with_timeoutand both
attach()methods have# Panics. - The CHANGELOG's
[Unreleased]section records thatBufferLaggednow reaches
callers ofget/get_with_timeout/try_get. grep -i channel aimdb-sync/README.mdreturns nothing outside the shutdown
plumbing, no removed API (set_with_timeout,*_with_capacity,try_set)
is documented as callable, and no latency or memory-footprint claim or
figure remains anywhere in the crate —README.md:444,446,src/lib.rs:135
andsrc/fork.rs:18-21included.make checkgreen, including the--no-default-featuresno_std build that
#205 established.
Revised 2026-08-15. Folded in the
src/lib.rs:133<50mslatency claim
(previously tracked nowhere) and the CHANGELOG note aboutBufferLagged
reaching callers (raised in the #208 review, never recorded). Measuring
aimdb-syncoverhead was considered and deferred indefinitely — no bench
target is planned, so this issue removes the claims outright and closes on its
own.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.