fix: prevent DAO state out-of-memory failures - #10
Conversation
helix-nine
left a comment
There was a problem hiding this comment.
This is a good diagnosis. The profiler evidence — 1.898 GB used against a 2 GB ceiling, failing in CanonicalEncoder / DaoStateMonitoringService.createDaoStateBlock, then surviving the same checkpoint at 4 GB with the same persisted state — nails it about as tightly as a memory bug can be nailed. It also rules out the Selkies-encoder theory I put in #9 as candidate (1), which is exactly what the diagnostics were for.
tsc --noEmit is clean on 22c88a2 and CI is green. Two things to fix before merge, though.
1. ram: 8192 is 8192 bytes, so the guard doesn't guard
hardwareRequirements.ram is compared in bytes, not MB. The chain, in start-core:
init.rs:393—let ram = get_mem_info().await?.total.0 as u64 * 1024 * 1024;(MebiBytes→ bytes) is what's stored as the server's RAM.registry/device_info.rs:195—HardwareInfo.ram: u64carries that value;system/mod.rs:352renders it withfn format_ram(bytes: u64).s9pk/v2/manifest.rs:168—if let Some(ram) = self.ram { if hw.ram < ram { return false } }. A raw comparison, no conversion.- The SDK passes it straight through:
buildManifestdoesram: manifest.hardwareRequirements?.ram || null.
So 8192 declares a minimum of 8 KiB, which every machine on earth satisfies. The README, instructions.md, and the release notes all promise an 8 GiB floor that nothing enforces.
That matters more than usual here because of what it's paired with. -Xmx4g is a fixed ceiling applied on every box, replacing a default that scaled with available memory — the 2 GiB you measured is consistent with the JVM's default MaxRAMPercentage of 25% on an 8 GiB machine. On a 4 GiB box the old default would have been ~1 GiB; the new one is 4 GiB on a machine that only has 4 GiB total, and the guard meant to keep the package off that box is inert. A clean Java OutOfMemoryError is a much better failure than a kernel OOM-kill of the container.
The fix:
hardwareRequirements: {
ram: 8 * 1024 ** 3, // 8589934592
},Worth knowing this isn't your mistake — hardwareRequirements.ram is undocumented in the packaging guide (only .device is covered), and the two other packages in the fleet that set it, cal-diy-startos (2048) and elements-startos (4096), have it wrong the same way. I'll raise that upstream separately.
One consequence to be deliberate about: once this actually enforces, boxes under 8 GiB stop being offered the package, including anyone already on 1.10.4:0. Given your evidence that DAO parsing needs ~1.9 GiB of heap, I think that's the right outcome — those boxes can't run this reliably and shouldn't silently install it — but it is a real change and worth calling out in the release notes.
2. The release notes drop everything 1.10.4 introduced
1.10.4:0 only ever reached community-beta, and :1 supersedes it — intermediate revisions get deindexed on promotion, so users coming from community-prod will go 1.10.3:4 → 1.10.4:1 in one hop and see only:
Increases the Bisq JVM heap limit to 4 GiB…
They'd never learn that this is the required 1.10.4 upstream security release, that the desktop moved from KasmVNC to Selkies, or — the operationally important one — that Bitcoin connection handling changed and local-only mode needs them to enable bloom filters. That last one has a critical task attached; the notes shouldn't be the place it goes unmentioned.
:1's notes should carry the full 1.10.4 story plus the heap fix. Something like:
Updated Bisq to 1.10.4, a required security update that strengthens validation of DAO
blocks, blind votes, disputes, witness signatures, and network data.
- Replaced KasmVNC with Selkies for the browser desktop
- Bisq uses Bitcoin's dedicated, trusted local peer connection by default, with an
explicit Bisq network fallback mode
- Raised the Bisq JVM heap limit to 4 GiB to prevent out-of-memory failures during DAO
state serialization
- Now requires at least 8 GiB of system memory
[Full upstream release notes](https://github.com/bisq-network/bisq/releases/tag/v1.10.4)
Localized in all five, and keep the upstream link — it was dropped in this revision.
Notes, non-blocking
JAVA_TOOL_OPTIONSapplies to every JVM started in the container and makes each one printPicked up JAVA_TOOL_OPTIONS: -Xmx4gto stderr. Harmless here since Bisq is the only JVM, just expect it in the logs.- Once the RAM floor is real,
-XX:MaxRAMPercentage=50would be equivalent on an 8 GiB box and self-scale upward on larger ones. A fixed-Xmx4gis fine given the measured ~1.9 GiB working set; only worth switching if you'd rather not revisit the number as DAO state keeps growing — which, given this bug, it will. - The Prettier reformat of
dependencies.tsis fine and you flagged it in the body.
|
Thanks both — the evidence here is conclusive. @martinbarilik's The Aug 8 soak — DAO stuck once, cleared by a resync, no heap error — meets the exit criterion @BeeJoe set for unblocking promotion: the OOM → incomplete hash chain → resync → OOM loop is broken. PR #10 still needs the two items from my review, neither of which is affected by any of the above: 1.
Upstream #7989's own "at least 8 GB RAM" recommendation makes the 8 GB floor exactly the right number — it just has to be written in bytes to take effect. Worth a line in the release notes that this gates installs on smaller hosts. 2. Release notes carry only the heap fix
With those two, this is good to merge to beta. |
|
@helix-nine Addressed both requested changes in dd9d445:
Validation completed successfully:
The non-blocking JAVA_TOOL_OPTIONS, fixed-Xmx, and formatting notes require no further changes. |
helix-nine
left a comment
There was a problem hiding this comment.
Both items verified on dd9d445 — approving.
hardwareRequirements.ramis8 * 1024 ** 3, and yours9pk inspectshowing8589934592confirms it survives into the built manifest, so the floor is real.- Release notes now carry the full 1.10.4 story in all five locales: the security update as the lead, Selkies, the local Bitcoin connection with its Bloom-filter prompt and fallback mode, the heap fix, the 8 GiB requirement and its consequence for smaller hosts, and the upstream link. A user coming from
1.10.3:4now gets the whole picture in one hop.
Also confirmed independently: CI green, tsc --noEmit and prettier --check clean, and your commits are signed this time — so no override needed to merge.
Two things worth carrying forward past the merge rather than losing here, neither blocking:
Upstream is still unfixed. bisq#7989 has no maintainer response and no post-1.10.4 commit touches canonical DAO encoding. -Xmx4g buys margin against a working set that grows with DAO state — 1.898 GB measured against a 4 GB ceiling is roughly 2x today, and that shrinks on its own. If it exhausts again, the answer isn't another heap bump; it's an upstream encoder change.
The DAO-wedge is a separate symptom. @martinbarilik's Aug 8 run had DAO get stuck once and need a manual resync, with no heap error — that's the intermittent behavior he described predating all of this, and it survives this fix. #9 closes with this PR correctly, since the OOM loop is what it tracked. If the wedge recurs on beta, it's worth its own issue rather than reopening this one.
Nice work chasing this all the way to the upstream root cause.
Closes #9.
Summary
JAVA_TOOL_OPTIONS=-Xmx4gfor the Bisq daemon so DAO state serialization is not constrained by the launcher’s effective 2 GiB heap.1.10.4:1and update localized release notes, developer documentation, and user instructions.startos/dependencies.ts.Validation
npm run checknpx prettier --check startosgit diff --checkmake x86start-cli s9pk inspect bisq_x86_64.s9pk manifestThe clean x86_64 build completed successfully at commit
22c88a2. The inspected artifact reports version1.10.4:1, x86_64 architecture, andhardwareRequirements.ram: 8192.Runtime verification
The x86_64 artifact was installed in place over the affected
1.10.4:0instance while preserving its existing Bisq data.1.10.4:1.Max memory: 4 GB; the previous package reported 2 GB.OutOfMemoryErroroccurred through the follow-up stability check. The prior 2 GB run failed eight seconds after the same checkpoint inCanonicalEncoder/DaoStateMonitoringService.createDaoStateBlock.This verifies the exact persisted-state path and timing that reproduced the issue. A longer soak remains prudent. Backup/restore and uninstall/reinstall were not repeated for this runtime-only change.