rfc: keep Piri's state in Postgres and blob store by bajtos · Pull Request #36 · fil-one/RFC

bajtos · 2026-10-21T13:29:29Z

Written by Claude.

Summary

Keep Piri's database-like state in Postgres. Move PDP aggregates from a local LevelDB onto the pdp_blob_pipeline rows, where they commit with the pipeline state and are deleted when AddRoots retires the batch. Split the IPNI publisher store so the chain head and per-context maps go to Postgres and the advert and entry blocks go to the blob store (FIL-1577). Leave the wallet keystore as an open question; its key is already copied into Postgres eth_keys in plaintext.

Piri keeps PDP aggregates and the IPNI advertisement chain in local LevelDB stores next to its Postgres database, so a consistent backup needs both. Move aggregates onto the pdp_blob_pipeline rows, split the publisher store between Postgres and the blob store, and record what we know about the wallet keystore as an open question. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

chatgpt-codex-connector · 2026-10-21T13:29:44Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-22T09:34:42.753674Z	`642ccd6`	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copilot

🔵 Needs a closer look

It is an architectural proposal involving a data migration, a known double-submission tradeoff, and an unresolved plaintext-key security question that require human consensus and sign-off per the repo's RFC process.

No open issues 🎉

What changed in this PR

This PR adds a new design RFC proposing that Piri keep its database-like state in Postgres instead of LevelDB. It argues that Postgres is already a hard dependency (the PDP pipeline runs on Curio's harmonytask using FOR UPDATE SKIP LOCKED) and that consolidating state yields one backup, one transaction boundary, and one place to debug. The document fits alongside the other proposal-stage RFCs in rfcs/, following the repo's naming and structure conventions.

Changes:

Moves PDP aggregates from LevelDB onto existing pdp_blob_pipeline rows via migration 00000008 (adds agg_position and inclusion_proof), so aggregate data commits with pipeline state and is deleted at AddRoots retirement.
Splits the IPNI publisher store: chain head and per-context maps go to Postgres; immutable adverts/entry chunks go to the blob store (FIL-1577).
Leaves the wallet keystore as an open question, noting the key is already copied into Postgres eth_keys in plaintext and listing options.

File	Description
rfcs/2026-10-piri-state-in-postgres.md	New RFC documenting motivation, background, migration/lifecycle design for PDP aggregates and the IPNI publisher store, alternatives, and the keystore open question.

I verified the filename prefix (2026-10-), H1 title, and **Status:** Proposal all match established conventions (several existing RFCs use the **Status:** Proposal form). Internal cross-references are consistent: migration 00000008, the new column names (agg_position, inclusion_proof), and the query shape (WHERE aggregate_root = $1 ORDER BY agg_position) line up across the Schema, Lifecycle, and Upgrade sections. The numeric estimates (per-blob proof size vs. the 30 GB–0.8 TB range at 12 PiB) are self-consistent. The only minor asymmetry is that storacha/piri#192 is cited inline but not in References while fil-forge/piri#183 is in References but not cited inline — this is within normal RFC reference curation and not a defect, so I did not file a comment. I found no objective defects to comment on.

_{Review effort: Balanced}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 641f5d04ed

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-10-21T13:35:36Z

+
+The LevelDB keystore holds the owner's wallet key as plaintext. At startup, `provisionPDPState` copies it into the Postgres table `eth_keys`, and Curio's transaction sender and `RegisterProvider` sign with that copy. After provisioning, the LevelDB keystore serves only a startup existence check and the `piri wallet` CLI. The key is therefore already in Postgres, which contradicts the `LocalOnlyModule` comment "private keys must never leave disk".
+
+Options:


Include the established OpenBao wallet design

For appliance deployments, these options omit the repository's existing key-management design: rfcs/2026-08-regional-security-and-key-management.md:34-40 specifies local OpenBao as the provider wallet's single home so secrets are protected at rest and a revoked appliance cannot restart. Treating custody as an unconstrained open question could therefore lead implementers to choose eth_keys, a file keystore, or LevelDB and violate that architecture; include the OpenBao path or explicitly explain that this RFC supersedes the earlier decision.

Useful? React with 👍 / 👎.

The implementation in fil-forge/piri#188 differs from the first draft in three details: the upgrade migration also clears agg_task_id so reset rows are claimed on the next idle poll, AddRoots is the only reader and hands the loaded aggregates to the piece acceptor, and the fold matches pieces to rows by blob digest because two blobs can share a commP. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

Replace the outline of the IPNI split with the design FIL-1577 will implement, and answer the review on atomic publishing and restores. - Keep the head and the per-context maps in Postgres, unchanged in behaviour, and store each advert as one blob-store object. - Inline entries chunks into identity CIDs instead of storing them. - Publish a batch in one transaction with batched SQL, and announce only after the commit. - Explain why restoring only Postgres keeps the chain consistent. - List the layouts considered and rejected. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

bajtos · 2026-10-22T09:27:38Z

@codex review

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

Copilot

🟡 Changes recommended

A minor tense/grammar error on line 19 ("split" should be "splits") should be corrected before approval.

1 open issue

Use present tense for the RFC split description · New

_{Review effort: Balanced}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 642ccd6055

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

alanshaw · 2026-10-22T09:39:20Z

+  requests the chunk by CID; Piri answers by decoding the CID, and the indexer's
+  hash check passes. Nothing in go-libipni or storetheindex rejects an identity
+  link. The cid.contact CAR mirror skips identity blocks and falls back to
+  fetching from Piri.


Clever, I like it. Maybe worth a PR to storetheindex to avoid the round trip.

frrist · 2026-10-22T09:48:54Z

Long overdue change, thanks for proposing and making it! At a high level, it looks great to me.

A review read "a release deletes nothing" as maps that outlive a removal advert, which would leave a re-accepted blob unindexed. Piri publishes no removal advert, so state that and what re-accept does. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

Aggregates live on pdp_blob_pipeline rows since the previous commit, so nothing reads the aggregator datastore any more. Delete its fx providers, the aggregator storage config and the aggregate Store type, so a node no longer creates data_dir/aggregator. Also delete the code this leaves unused: internal/ipldstore, whose only user was the aggregate store, and the test-only LevelDB object store. The docs drop the aggregator job-queue database that the move to harmonytask already retired, and tell operators they can delete the old directory. Refs: fil-one/RFC#36 Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

_Written by Claude._ Fixes FIL-1577. Design: fil-one/RFC#36. Uses go-ipni-tools `main` (7d9dcd4), which has the batch store from fil-forge/go-ipni-tools#27. ## Summary The IPNI advertisement chain moves out of the LevelDB at `data_dir/publisher`. The head and the per-context maps are rows in Postgres and commit in the transaction that publishes a batch. Adverts are blob-store objects, and entries chunks of up to 10 multihashes are inlined into identity CIDs. ```diff pkg/store/ +├── ipnistore/ # go-ipni-tools store on ipni_head, ipni_contexts and an object store └── objectstore/ + └── prefixed.go # ipni/ prefix in the PDP bucket pkg/curiopdp/schema/sql/ +└── 00000008-ipni-publisher-store.sql ``` | Record | Where | |---|---| | Signed head | `ipni_head`, one row, compare-and-swap | | Context → entries chunk, context → metadata | `ipni_contexts` | | Advert | `ipni/<cid>` in the S3 bucket, or flatfs at `data_dir/ipni` | | Entries chunk of up to 10 multihashes | inlined into the advert's identity CID link | ```text PublishClaimed (one harmonytx transaction) load claimed rows PublishBatch LoadContexts: one SELECT of both maps, context_id = ANY($1) PutAdverts: adverts to the object store, 16 at a time CommitBatch: one unnest upsert of both maps head compare-and-swap retire the batch's queue rows AfterCommit: announce the new head (ipni.announce span, one HTTP PUT span per indexer) ``` The head, the maps and the queue retirement commit together, so a failed batch leaves everything as it was and its rows stay claimed for the retry. Announcing after the commit means an indexer never hears of a head a rollback discarded. Memory mode keeps go-ipni-tools' datastore store, with identity entries. ## Evidence - `TestPublishClaimedAnnouncesTheNewHead` (in-memory): one announce carrying the new head. - `TestPublishClaimedTracesTheAnnounce`: the request to the indexer is an `HTTP PUT` span under `ipni.announce`; it fails when the sender uses an untraced client. - `TestPutAdverts`: every advert written; a failed write fails the batch. - `pkg/fx/app` graph validation passes for the memory, filesystem and S3 setups (run on Linux with `-tags=skiff`). - Postgres-only, passing locally on macOS with Docker: `TestHead`, `TestContexts`, `TestLoadAndCommitContexts` (records round-trip, replace, delete, and a failed head swap writes none), `TestPostgresStore_IndexerSyncsPublishedChain` (ipnisync fetches every advert and its identity chunk through `/ipni/v1/ad/`), `TestPostgresStore_FailedAdvertWriteRollsBack` (head, maps and queue rows unchanged, no announce). - `BenchmarkPublishBatch` (Postgres in Docker, Apple M4 Pro, 3 runs each) publishes one-multihash specs inside one transaction, as `PublishClaimed` does: | Adverts | Per-item store calls | Batch store | Speed-up | |--:|--:|--:|--:| | 100 | 67 ms | 10 ms | 6.4× | | 1,000 | 625 ms | 61 ms | 10.2× | Memory per batch is unchanged (41 MB and 414 MB) and allocations drop by 15%. - smelt end to end (S3 mode, storetheindex, commit 62cb81b): after a guppy upload, the indexer fetched both adverts and both identity entries chunks (`bafyqa…`) through `/ipni/v1/ad/` with 200, logged "Indexed multihashes from chunk" with `count=1` for each, and the finder returns Piri's provider record for the shard. `ipni_head` holds one head, `ipni_contexts` two rows, the queue is empty, the adverts are under `piri-0-pdp/ipni/`, and no `publisher/` directory exists. ## Merge Danger **Door:** one-way Nodes start a new, empty chain; `data_dir/publisher` is no longer read and can be deleted. No dev or staging node announces to IPNI, so nothing is migrated. **Blast Radius:** IPNI Publishing and serving adverts change on every node. The migration takes `00000008`, which #188 also uses; whichever merges second renumbers. --------- Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

Aggregates live on pdp_blob_pipeline rows since the previous commit, so nothing reads the aggregator datastore any more. Delete its fx providers, the aggregator storage config and the aggregate Store type, so a node no longer creates data_dir/aggregator. Also delete the code this leaves unused: internal/ipldstore, whose only user was the aggregate store, and the test-only LevelDB object store. The docs drop the aggregator job-queue database that the move to harmonytask already retired, and tell operators they can delete the old directory. Refs: fil-one/RFC#36 Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

_Written by Claude._ Store each PDP aggregate on the `pdp_blob_pipeline` rows of its pieces instead of the local LevelDB aggregate store. The aggregation task wrote that store outside its Postgres transaction and never deleted from it, so it grew without bound, and a lost record stalled AddRoots. RFC: fil-one/RFC#36. ## Changes - Migration `00000009` adds `agg_position` (the piece's index in the aggregate) and `inclusion_proof` (CBOR `merkletree.ProofData`) to `pdp_blob_pipeline`, and a partial index on `(aggregate_root, agg_position)` for the lookups AddRoots makes by root. - The aggregation fold sets both columns next to `aggregate_root` in its transaction, with one `UPDATE ... FROM unnest(...)` per aggregate. It matches each piece to its row by digest, because two blobs can share a piece. It rejects a blob given for two pieces, and when the update matches fewer rows than the aggregate has pieces, a second query in the same transaction names each unmatched piece and its cause: no row, a row holding another piece, or a row already aggregated. - `loadAggregate` rebuilds the aggregate from `SELECT DISTINCT ... WHERE aggregate_root = $1 ORDER BY agg_position`. Two blobs whose shared piece is an aggregate on its own fold into two occurrences of one root, and `DISTINCT` loads them as one. It rejects gaps in the positions and two different pieces at one position. AddRoots loads each aggregate once and passes the loaded aggregates to `PieceAcceptor`, so the acceptor takes no store or loader at all (`manager` cannot import `pipeline` without a cycle). - The existing AddRoots retirement `DELETE` removes the aggregates with their rows; it is factored into `retire` so a test can exercise it. - `docs/content/concepts/database.md` says where aggregates live and what the upgrade does, and that each Piri instance needs its own PostgreSQL database. Nothing consumes `aggtypes.Store` any more, and fx constructs providers lazily, so the node no longer opens `<data_dir>/aggregator`. The `aggregator_datastore` providers, `AggregatorStorageConfig` and `types/store.go` stay in place for the next PR in the stack to remove. ## Upgrade The migration clears `aggregate_root` and `aggregated_at` on unretired rows and empties `pdp_root_submissions`, so those blobs fold into new aggregates. It also clears `agg_task_id`: the aggregation task's `IAmBored` claims only rows without one, and `followCommP` has long since run for their commP tasks, so a reset row that kept its task ID would wait until another piece spawned a fold. A `PDPAddRoots` task that had claimed some of the deleted submission rows finds none on its next run and completes as done. A batch that AddRoots landed on chain before the upgrade but did not retire is added a second time; dev and staging hold no production data, so that is acceptable. ## Tests The new tests use `testutil.NewHarmonyDB`, which needs Docker and skips on darwin in CI: - The aggregate rebuilt from the rows equals the one the fold computed, with order and proofs intact, including two blobs sharing a piece. - Two blobs whose shared piece is an aggregate on its own fold into two occurrences of one root, which load as one. - Rows that put two different pieces at one position fail to load. - `storeAggregate` names the cause for a blob given for two pieces, a missing row, a row holding another piece, and a row already aggregated. - Retirement leaves no aggregate to load. - `PDPAddRoots` with no submission rows completes. - `IAmBored` claims a reset row with `agg_task_id` cleared, and leaves one with a stale `agg_task_id`. `NewHarmonyDB` applies every migration to an empty database, so the test covers the state the reset leaves rather than running the SQL against old data. --------- Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

_Written by Claude._ Remove the LevelDB aggregator datastore. Since #188, aggregates live on `pdp_blob_pipeline` rows and nothing reads the old store, so this PR only deletes code. It follows [fil-one/RFC#36](fil-one/RFC#36). - Delete the `aggregator_datastore` fx providers (filesystem, local-only and memory modules), `AggregatorStorageConfig` and its path in `repo.go`, and the aggregate `Store` type. A node no longer creates `data_dir/aggregator`. - Delete `internal/ipldstore`, whose only user was the aggregate store, and the test-only LevelDB object store. `syndtr/goleveldb` becomes an indirect dependency; Lotus still requires it. - In `docs/content/concepts/database.md`, drop the aggregator job-queue database that the move to harmonytask already retired, and tell operators they can delete `data_dir/aggregator` after upgrading. Stacked on #188. ## Testing - `go build`, `go vet` and `go test` for `pkg/fx/app` (all three storage variants), `pkg/pdp/...`, `pkg/config/...` and `pkg/store/objectstore`. - The MinIO object store tests need Docker and did not run locally. Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

Copilot AI balanced review requested due to automatic review settings October 21, 2026 13:29

Copilot started reviewing on behalf of bajtos October 21, 2026 13:30

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Oct 21, 2026

View reviewed changes

bajtos mentioned this pull request Oct 21, 2026

feat(pdp): store aggregates in pdp_blob_pipeline fil-forge/piri#188

Merged

bajtos mentioned this pull request Oct 21, 2026

refactor: remove the LevelDB aggregator datastore fil-forge/piri#189

Merged

This was referenced Oct 21, 2026

feat: identity entries and batched publish fil-forge/go-ipni-tools#23

Closed

feat(ipni): keep the advertisement chain in Postgres fil-forge/piri#191

Merged

feat: batch store reads and writes in PublishBatch fil-forge/go-ipni-tools#24

Closed

bajtos added 2 commits October 22, 2026 11:21

clean up the text

abce2cf

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

more tweaks

642ccd6

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

bajtos requested review from alanshaw and a balanced review from Copilot October 22, 2026 09:27

bajtos changed the title ~~rfc: keep Piri's state in Postgres~~ rfc: keep Piri's state in Postgres and blob store Oct 22, 2026

Copilot started reviewing on behalf of bajtos October 22, 2026 09:28

View session

fix title

fd450d1

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Comment thread rfcs/2026-10-piri-state-in-postgres.md Resolved

chatgpt-codex-connector Bot reviewed Oct 22, 2026

View reviewed changes

Comment thread rfcs/2026-10-piri-state-in-postgres.md Resolved

Comment thread rfcs/2026-10-piri-state-in-postgres.md Resolved

alanshaw approved these changes Oct 22, 2026

View reviewed changes

bajtos added 2 commits October 22, 2026 11:53

rfc: fix tense in the list of LevelDB stores

1a336ca

Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

bajtos merged commit d6aac6d into main Oct 22, 2026
2 checks passed

Navigation Menu

Repository navigation

rfc: keep Piri's state in Postgres and blob store - #36

rfc: keep Piri's state in Postgres and blob store#36
bajtos merged 8 commits into
mainfrom
piri-state-in-postgres

bajtos commented Oct 21, 2026 •

edited by github-actions Bot

Loading

Uh oh!

chatgpt-codex-connector Bot commented Oct 21, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Oct 21, 2026

Uh oh!

Uh oh!

Uh oh!

bajtos commented Oct 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

alanshaw Oct 22, 2026

Uh oh!

frrist commented Oct 22, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants


		The LevelDB keystore holds the owner's wallet key as plaintext. At startup, `provisionPDPState` copies it into the Postgres table `eth_keys`, and Curio's transaction sender and `RegisterProvider` sign with that copy. After provisioning, the LevelDB keystore serves only a startup existence check and the `piri wallet` CLI. The key is therefore already in Postgres, which contradicts the `LocalOnlyModule` comment "private keys must never leave disk".

		Options:

Conversation

bajtos commented Oct 21, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

chatgpt-codex-connector Bot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bajtos commented Oct 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

alanshaw Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

frrist commented Oct 22, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bajtos commented Oct 21, 2026 •

edited by github-actions Bot

Loading

chatgpt-codex-connector Bot commented Oct 21, 2026 •

edited

Loading