PDP #22

hannahhoward · 2024-11-04T10:27:22Z

Goals

Implement PDP through curio

Implementation

The basic strategy here is as follows:

When PDP is present, it replaces a number of parts of the blob service, as they now happen directly through curio. (I actually toyed with mirroring the Access and Presigner APIs but it was a little awkward
The big chunk of new logic is the aggregator, which is kind of like a mini-w3filecoin pipeline but a lot simpler. I've built this with the goal of making it runnable on SQS queues just like the w3filecoin pipeline, or runnable locally with the job queue. The aggregator can be viewed in terms of its constituent packages:
1. pkg/pdp/aggregator/aggregate handles calculating aggregate roots and assembling piece proofs (it's like a mini go-data-segment
2. pkg/pdp/aggregator/fns contains the core logic of each aggregation step, minutes side effects/state
3. pkg/pdp/aggregator/steps.go contains the actual steps in aggregation with side effects
4. pkg/pdp/aggregator/local.go contains the implementation of aggregation that can run on a native piece of hardware. AWS version to come later
PieceAdder is another function that handles essentially saying to Curio "I have a new piece to upload" and Curio responds with an upload URL -- it's relatively analogous to our blob/allocate actually (except we do it in this node)
PieceFinder gets the piece cid from the blob hash and size. Importantly, this is probably the ugliest bit of logic for now cause Curio isn't currently setup for reliable read on write, so there is a change we will get blob/accept BEFORE we Curio is ready to give us the piece cid -- that's why there's the retry loop in there for now
pkg/pdp/curio is just the curio client ot make http calls to curio
I also implemented the ReceiptStore cause I needed it
I also added a utility for storing ipld types in a data store -- see pkg/internal/ipldstore -- I think this will be useful in future
There are essentially one new invocation available on the UCAN endpoint - pdp/info which you can use to get the state of your aggregation and the inclusion proof. This may still need some changes on auth though? Not sure
pdp/accept is a self issued invocation used by the server to track when PDP is done and save results.

The PR depends on the following:
storacha/go-ucanto#31
storacha/go-capabilities#2
storacha/go-capabilities#3
as well as two new repos:
https://github.com/storacha/go-piece
https://github.com/storacha/go-jobqueue (extracted from indexing-service)

alanshaw

Got through about half - have left some minor feedback. Need to 💤

cmd/main.go

alanshaw · 2024-11-04T23:02:14Z

cmd/main.go

+							return errors.New("pdp-proofset must be set if curio is used")
+						}
+						curioAuth := cCtx.String("curio-auth")
+						proofSet := cCtx.Int64("pdp-proofset")


Use cCtx.Uint64?

pkg/pdp/aggregator/aggregate/aggregate.ipldsch

pkg/pdp/aggregator/fns/fns.go

alanshaw · 2024-11-04T23:27:11Z

pkg/pdp/aggregator/fns/piecessofar.ipldsch

+	TotalSize           uint64
+	ReverseSortedPieces []PieceLink


Suggested change

TotalSize uint64

ReverseSortedPieces []PieceLink

totalSize Int

reverseSortedPieces []PieceLink

alanshaw · 2024-11-04T23:31:27Z

pkg/pdp/aggregator/fns/fns.go

+}
+
+// PiecesSoFar tracks in progress work building an aggregation
+type PiecesSoFar struct {


We call this a "buffer" in current filecoin pipeline...IDK maybe worth naming it something along those lines?

pkg/pdp/aggregator/fns/fns.go

alanshaw · 2024-11-10T12:40:32Z

pkg/pdp/aggregator/fns/fns.go

+}
+
+// InsertReverseSortedBySize adds a piece to a list of pieces sorted largest to smallest, maintaining sort order
+func InsertReverseSortedBySize(sortedPieces []piece.PieceLink, newPiece piece.PieceLink) []piece.PieceLink {


Minor but something about "reverse sorted" feels off. Is there some sort of assumption baked in there about what "forwards" is? I've often seen "ascending" and "decending" be used to describe sort order. InsertOrderedBySizeDescending/InsertOrderedBySizeDesc?

Personally I'd also switch other instances of "sorted" to "ordered" as it describes the desired state rather than the fact that a sort operation took place prior.

makes sense

alanshaw · 2024-11-10T12:43:15Z

pkg/pdp/aggregator/fns/fns.go

+}
+
+// MinAggregateSize is 128MB
+// Max size is 256MB -- this means we will never see an individual piece larger


Can we get clarification here on why this is? The default is 128mb in our client, but it is configurable and we say max size is 4gb (or thereabouts). So please note that this would be a breaking change if it's now a requirement...

this is a question for @magik6k and a curio thing. we can sort it later.

pkg/pdp/aggregator/fns/fns.go

alanshaw

I don't think I have any blocking feedback here. I think it's fine for now, but the current way it is integrated could use a clean up.

I'm interested in your thoughts on the blob/retain idea (see comments) as it feels like a part we're missing in the flow and might allow PDP to land here quite nicely IMHO.

pkg/service/storage/ucan.go

pkg/pdp/pieceadder/curioadder.go

alanshaw · 2024-11-10T14:43:41Z

pkg/pdp/aggregator/steps.go

+		}
+		aggregates = append(aggregates, aggregate)
+	}
+	// TODO: Should we actually send a piece accept invocation? It seems unneccesary it's all the same machine


I believe we only do this in w3up to ensure invocations and their receipts are automatically stored in a bucket.

ah something to consider for future. receipts end up stored already

alanshaw · 2024-11-10T14:45:42Z

pkg/pdp/aggregator/steps.go

+		return fmt.Errorf("generating receipts: %w", err)
+	}
+	for _, receipt := range receipts {
+		if err := pa.receiptStore.Put(ctx, receipt); err != nil {


Might want to parallelize/batch this - an average aggregate right now has over 4,000 pieces.

256 mb should at least be less :)

Oh, yes of course...way smaller aggregates.

alanshaw · 2024-11-10T14:48:37Z

pkg/service/storage/ucan.go

+						if err != nil {
+							log.Errorf("adding to pdp service: %w", err)
+							return blob.AllocateOk{}, nil, failure.FromError(err)
+						}


Non-blocking - it would be nice if we could do this without conditionals. There must be a common interface we can fit...

welcome to golang -- we could use our Wrap and Unwrap on our result type... but I don't know if it ends up being worth it.

pkg/service/storage/ucan.go

alanshaw · 2024-11-10T15:10:43Z

pkg/service/storage/ucan.go

+						// get a download url
+						loc = storageService.PDP().PieceFinder().URLForPiece(piece)
+						// submit the piece for aggregation
+						err = storageService.PDP().Aggregator().AggregatePiece(ctx, piece)


What happens when a piece in the aggregate is removed? We have to re-issue receipts right?

alanshaw · 2024-11-10T16:01:31Z

pkg/service/storage/ucan.go

+							return blob.AcceptOk{}, nil, failure.FromError(err)
+						}
+						// generate the invocation that will complete when aggregation is complete and the piece is accepted
+						pieceAccept, err := pdp.PDPAccept.Invoke(


Musing on this - what if we expose a more generic blob/retain that can be used for both? PDP is not really an operation. What I think we need is an instruction for the blob to continue to be stored over time...and the receipt for an invocation to do this could be details of how to prove it is continually being stored. i.e. a retention receipt would specify the proof type is PDP and contains the inclusion proof. The retention reciept for non-PDP is just "trust me I have it" or "test me with station".

The dictionary definition of "retain" is "continue to have (something); keep possession of".

Side note: perhaps the invocation could include accepted retention policies?

After a blob has been accepted (delivered), the upload service would issue a blob/retain invocation to ensure the blob continues to persist on the storage node. Of course for PDP the invocation may have some async tasks to perform (the aggregation), so perhaps simply responds with a blob/retain/start effect (not sold on that name).

We can surface this to users by adding the blob/retain/start fx to the original blob/add invocation.

"retain" also has a nice antonym "release", which we might use to stop the node from submitting PDP proofs (and allow it to garbage collect).

to be clear, pdp/accept here is is a LATER thing -- it's saying, this will be done when I've stored successfully in PDP.

Happy to iterate on naming -- I don't love all this as it is.

implements a complete pdp service with aggregation, and invocations to track progress. replaces most of blob service when present

Co-authored-by: ash <[email protected]>

…t library

Co-authored-by: ash <[email protected]>

alanshaw reviewed Nov 4, 2024

View reviewed changes

alanshaw reviewed Nov 10, 2024

View reviewed changes

alanshaw approved these changes Nov 10, 2024

View reviewed changes

hannahhoward and others added 19 commits November 11, 2024 10:41

feat(piece): add utilities for piece hashes

2cb1c31

build(go.mod): mod tidy

c37957a

feat(curio/client): add curio API client

ea933f4

fix(client): add full piece hash setup

25e1c44

feat(pdp): implement pdp

31aa7be

implements a complete pdp service with aggregation, and invocations to track progress. replaces most of blob service when present

fix(deps): fix tests

c131da9

Update cmd/main.go

63796ba

Co-authored-by: ash <[email protected]>

Update pkg/pdp/aggregator/aggregate/aggregate.ipldsch

634eed1

Co-authored-by: ash <[email protected]>

refactor(pdp): rename piecessofar to buffer

e492aee

feat(pdp): major testing and fixes to PDP, refactor client, and clien…

516766e

…t library

Update pkg/pdp/aggregator/fns/fns.go

2eb7bbd

Co-authored-by: ash <[email protected]>

Update pkg/pdp/aggregator/fns/fns.go

f735507

Co-authored-by: ash <[email protected]>

Update pkg/service/storage/ucan.go

4f110a3

Co-authored-by: ash <[email protected]>

Update pkg/service/storage/ucan.go

037761a

Co-authored-by: ash <[email protected]>

Update pkg/pdp/pieceadder/curioadder.go

29a151f

Co-authored-by: ash <[email protected]>

Update pkg/service/storage/ucan.go

5afdbf9

Co-authored-by: ash <[email protected]>

fix(mod): go mod tidy

27162d0

refactor(pdp): rename function

38b24ae

style(fmt): go fmt

d9e345e

hannahhoward force-pushed the feat/pdp branch from f7f6c81 to d9e345e Compare November 11, 2024 03:58

hannahhoward and others added 4 commits November 11, 2024 11:12

test(ucan): fix ucan test

ca05fbb

style(lint): fix lint errors

73e327a

fix(mod): use go-capabilities commit from main

58b8614

Update pkg/pdp/aggregator/fns/fns.go

00eaae3

Co-authored-by: ash <[email protected]>

hannahhoward merged commit f40c1cc into main Nov 11, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PDP #22

PDP #22

hannahhoward commented Nov 4, 2024

alanshaw left a comment

alanshaw Nov 4, 2024

alanshaw Nov 4, 2024

alanshaw Nov 4, 2024

hannahhoward Nov 9, 2024

alanshaw Nov 10, 2024 •

edited

Loading

hannahhoward Nov 11, 2024

alanshaw Nov 10, 2024

hannahhoward Nov 11, 2024

alanshaw left a comment

alanshaw Nov 10, 2024

hannahhoward Nov 11, 2024

alanshaw Nov 10, 2024

hannahhoward Nov 11, 2024

alanshaw Nov 11, 2024

alanshaw Nov 10, 2024

hannahhoward Nov 11, 2024

alanshaw Nov 10, 2024

alanshaw Nov 10, 2024 •

edited

Loading

hannahhoward Nov 11, 2024

PDP #22

PDP #22

Conversation

hannahhoward commented Nov 4, 2024

Goals

Implementation

alanshaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanshaw Nov 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanshaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanshaw Nov 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanshaw Nov 10, 2024 •

edited

Loading

alanshaw Nov 10, 2024 •

edited

Loading