Handle quota and cleanup correctly by dryajov · Pull Request #103 · durability-labs/archivist-node

dryajov · 2026-03-17T17:51:13Z

This PR fixes many issues related to quota handling and dataset cleanup, in addition to bumping the block size to 512kb.

…ntly

…ter all puts finish

storeManifest bypassed putBlockInternal and wrote directly to repoDs, missing the onBlockStored callback. The block exchange engine never learned about the manifest block, causing remote downloads to time out.

…lot data, to avoid dropping slots while proving in process

markspanbroek

Are all your changes going to be this big from now on? 😅

markspanbroek · 2026-04-02T11:02:41Z

+      # Add buffer past contract end so overlay data survives through the
+      # last proof window. Buffer covers period (last proof window) +
+      # proofTimeout (challenge window). Without this, maintenance can drop
+      # the overlay while the proving loop is still running its last proof.


I don't think that this is necessary. When the request has ended, submitting the proof will not work anyway. So it doesn't matter that generating the proof fails.

yeah, as per our discussion, this is mostly harmless on the marketplace side, but it will trigger re-downloads of data that is potentially already gone, so it will either download something that is not longer needed, or get stuck in retries.

markspanbroek · 2026-04-02T11:06:27Z

    let periodAtFinish = periodicity.periodOf(StorageTimestamp.init(clock.now()))
    if periodAtFinish != provingPeriod:
-      warn "Failed to generate initial proof in time",
+      warn "Initial proof generated after period rollover",


I would like to keep the word "failed" in there. If the proof is not generated in time, it will submit it in the next period, and it will therefore be rejected by the smart contract, because the challenge has changed.

Same for proving.nim

markspanbroek · 2026-04-02T11:55:27Z


  trace "Deleting leaf and block metadata"

+  await self.deletingLock.drain(treeCid)


It looks as though you're trying to ensure that no-one is writing to the overlay by draining the deletingLock barrier. As far as I can tell, this is susceptible to a race condition.

In putLeafBlockMetaImpl(), you're first checking whether the overlay status equals Deleting before entering the barrier.

But here you first drain the barrier, then call several async operations, and only towards the end you set the overlay status to Deleting. In the mean time putLeafBlockMetaImpl() could have entered the barrier again.

What the "barrier/inverse-semaphore" is doing here, is coordinating concurrent puts and deletes, the deleting flag is just an early exit marker for subsequent callers. But you're right that it might be safer to set it early.

markspanbroek · 2026-04-02T12:22:39Z

+import pkg/archivist/marketplace/purchasing
+import pkg/archivist/marketplace/purchasing/purchase
+import pkg/archivist/marketplace/sales
+import pkg/archivist/marketplace/availability/store
+import pkg/archivist/marketplace/storageinterface
+import pkg/archivist/marketplace/node {.all.}


This depends on a lot of internal details of the marketplace, which makes these tests rather fragile and complex. The marketplace interface should be relatively stable, and it seems that you only need for a call to marketplace.purchasing.purchase() to succeed, so I think it would make sense to introduce a simple mock for MarketplaceNode.

Not really, this is the only place were we can test every component in unison, introducing a mock defeats the purpose of this test that has in practice proven to be extremely useful at identifying issues early.

markspanbroek · 2026-04-02T12:29:08Z

 proc start*(builder: NodeBuilder): Future[Node] {.async.} =
  var arguments: seq[string]
  arguments.add("--disc-port=" & $(await builder.discoveryPortResolved))
+  arguments.add("--nat=none") # don't need nat for integration tests


Best not make this change here, I handled this in #110:

archivist-node/tests/testbed/builders/node.nim

Line 253 in c408d6b

arguments.add("--nat=" & builder.natResolved)

this shaves off around 10 minutes from the integration tests, otherwise it tries to initialize it on every startup.

Co-authored-by: markspanbroek <mark@spanbroek.net>

dryajov force-pushed the feat/bump-block-size branch from ca30d2e to 17f6893 Compare March 18, 2026 04:52

dryajov marked this pull request as ready for review March 18, 2026 20:58

dryajov force-pushed the feat/bump-block-size branch 8 times, most recently from b290697 to ebd1b41 Compare March 24, 2026 04:57

dryajov changed the title ~~Bump block size to 512kb~~ Handle quota and cleanup correct Mar 31, 2026

dryajov changed the title ~~Handle quota and cleanup correct~~ Handle quota and cleanup correctly Mar 31, 2026

dryajov mentioned this pull request Mar 31, 2026

[BUG] Uploads can exceed quota #109

Open

dryajov added 17 commits March 31, 2026 15:58

feat: set block size to 512kb

f387b89

feat: dissable nat in integration tests - speeds up startup significa…

d229be8

…ntly

feat: regenerate circuits for 512kb blocks

83f9737

feat: bump deps

a09fda1

feat: update circuits

7a3c456

feat: allow setting namespace for kubernetes

c6b38cc

fix: don't double del manifest and count against quota

bd7f260

feat: adding quota checks

b522a17

fix: remove noisy trace

437a85f

fix: properly handle verifiable manifest cleanup

f519b43

fix: race condition in concurrent put/drop test

34d0e07

feat: cleanup slots on failed purchases

9570bb2

feat: adding tracked semaphore (keyed)

2097a25

feat: avoid interleaving delets and puts by allowing delets to run af…

de865a8

…ter all puts finish

fix: deletel re-entrancy bug

b410009

feat: track cleanup future

29eb35d

feat: check for file presence

2a91f7d

dryajov added 21 commits March 31, 2026 15:58

fix: don't create a semaphore for count

cd75a75

feat: add asyncbarrier

48b1422

feat: move repo operations to operations to avoid put/del interleaving

391b135

feat: add cancellation tests

1ce13ff

fix: properly store and cleanup all manifests

ee18143

fix: remove un-used param

6e482ca

feat: trest proper cleanup across all manifests

70dbc86

fix: femove unused param

a5e55d4

fix: avoid mutating query iterator

48632f1

feat: allow setting dataset params with storeVerifiableManifest

9c92b06

bump archivist contracts

f30f964

bump circom_witnessgen

9829741

feat: use fetchManifest instead of manually decoding from block

35eb60a

feat: properly track manifest refcounts to avoid dangling manifests

d63c2aa

fix: fire onBlockStored callback when storing manifest block

fb1b337

storeManifest bypassed putBlockInternal and wrote directly to repoDs, missing the onBlockStored callback. The block exchange engine never learned about the manifest block, causing remote downloads to time out.

feat: add tests for manifest tracking

6237338

feat: streamline manifest tracking

b51a402

fix: skip non-manifest blocks

0f138c5

feat: more tightening of manifest tracking

246ee63

feat: make sure slot proving times out eventually and add buffer to s…

cdc58cc

…lot data, to avoid dropping slots while proving in process

fix: properly handle quota over limit uploads

8316dd0

dryajov force-pushed the feat/bump-block-size branch from 86b49fa to 8316dd0 Compare April 1, 2026 22:22

markspanbroek approved these changes Apr 2, 2026

View reviewed changes

fix: avoid double counting on update and set expiry

1c000f6

dryajov force-pushed the feat/bump-block-size branch from bbecc05 to 1c000f6 Compare April 2, 2026 19:25

dryajov and others added 3 commits April 2, 2026 12:59

Update archivist/marketplace/sales/states/initialproving.nim

a7d1701

Co-authored-by: markspanbroek <mark@spanbroek.net>

fix: potential race condition during delete

dd52bab

fix: minor review cleanup

819c4a2

dryajov merged commit 40b3bd4 into main Apr 3, 2026
35 checks passed

dryajov deleted the feat/bump-block-size branch April 3, 2026 14:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle quota and cleanup correctly#103

Handle quota and cleanup correctly#103
dryajov merged 42 commits intomainfrom
feat/bump-block-size

dryajov commented Mar 17, 2026 •

edited

Loading

Uh oh!

markspanbroek left a comment

Uh oh!

markspanbroek Apr 2, 2026

Uh oh!

dryajov Apr 2, 2026

Uh oh!

Uh oh!

markspanbroek Apr 2, 2026

Uh oh!

markspanbroek Apr 2, 2026

Uh oh!

markspanbroek Apr 2, 2026

Uh oh!

dryajov Apr 2, 2026

Uh oh!

markspanbroek Apr 2, 2026

Uh oh!

dryajov Apr 2, 2026 •

edited

Loading

Uh oh!

markspanbroek Apr 2, 2026

Uh oh!

dryajov Apr 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		trace "Deleting leaf and block metadata"

		await self.deletingLock.drain(treeCid)

Conversation

dryajov commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markspanbroek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dryajov Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dryajov Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dryajov commented Mar 17, 2026 •

edited

Loading

dryajov Apr 2, 2026 •

edited

Loading

dryajov Apr 2, 2026 •

edited

Loading