Backport index and ipni pdp tasks to PDPv0 #670
Conversation
rjan90
added
the
team/fs-wg
| -- To enable separation of access based on storage service we create a separate provider identity | ||
| -- for PDP IPNI requests. This is a practical concern for current IPNI operations with limited resources. | ||
| CREATE TABLE ipni_pdp_peerid ( | ||
| singleton BOOLEAN NOT NULL DEFAULT TRUE PRIMARY KEY CHECK (singleton = TRUE), | ||
| priv_key BYTEA NOT NULL, | ||
| peer_id TEXT NOT NULL UNIQUE | ||
| ); | ||
|
|
There was a problem hiding this comment.
This needs to go in a new file. My suggestion would be not create a new table. MK2.0 also uses the existing table.
| ALTER TABLE pdp_piecerefs ADD COLUMN indexing_task_id BIGINT DEFAULT NULL; | ||
| ALTER TABLE pdp_piecerefs ADD COLUMN needs_indexing BOOLEAN DEFAULT FALSE; | ||
| ALTER TABLE pdp_piecerefs ADD COLUMN ipni_task_id BIGINT DEFAULT NULL; | ||
| ALTER TABLE pdp_piecerefs ADD COLUMN needs_ipni BOOLEAN DEFAULT FALSE; |
There was a problem hiding this comment.
This would need to go into a new file. Always expect the existing SQL files are executed and will not be executed again.
There was a problem hiding this comment.
This task will only produce IPFS style ads. So, there would be no IPNI lookup path for pieces. Which was a requirement confirmed by Jen and Miro both in Dubai.
There was a problem hiding this comment.
+1 That's fine for now. I just want to get ipfs adds working first.
There was a problem hiding this comment.
piece CIDs into IPNI is lower priority; if it can be bundled into the same work without too much effort then that's great, but it doesn't buy us very much so it can be treated as a stretch goal
| } | ||
|
|
||
| pi := &PdpIpniContext{ | ||
| PieceCID: pcid, |
There was a problem hiding this comment.
Shouldn't this be pieceCID v2?
tasks/indexing/task_pdp_ipni.go
Outdated
| // The ipni provider key for pdp is at spid = 0 | ||
| func (P *PDPIPNITask) initProvider(tx *harmonydb.Tx) error { | ||
| var privKey []byte | ||
| err := tx.QueryRow(`SELECT priv_key FROM ipni_peerid WHERE spid = 0`).Scan(&privKey) |
There was a problem hiding this comment.
Let's not use sp_id 0 for consistency, we are keeping negative IDs for all non PoRep stuff. V2 has taken -1, so let's go with -2 here so we don't end up mixing 2 ad chains later when SP upgrade.
|
@ZenGround0 : will https://github.com/filecoin-project/curio/blob/feat/index-and-ipni/deps/config/types.go#L116 be updated to include filecoinpin.contact, or will that be somewhere else? |
| select { | ||
| case recs <- indexstore.Record{ | ||
| Cid: blockMetadata.Cid, | ||
| Offset: blockMetadata.Offset, |
There was a problem hiding this comment.
| Offset: blockMetadata.Offset, | |
| Offset: blockMetadata.SourceOffset, |
See https://pkg.go.dev/github.com/ipld/go-car/v2#BlockMetadata, this is important to support CARv2 which has a header before the internal CARv1 container.
But I see this is just copied straight from tasks/indexing/task_indexing.go and the same use exists above in tasks/indexing/task_pdp_ipni.go and where it's copied from in tasks/indexing/task_ipni.go. So @LexLuthr you're going to need to fix them too.
The Boost version of this is a good reference fwiw, (that you were responsible for @LexLuthr): filecoin-project/boost#1739 (see piecedirectory/piecedirectory.go parseRecordsFromCar).
I also note that we're not supporting PoDSI here; something to look into later maybe.
There was a problem hiding this comment.
I think we filter out cid.contract for anything not mainnet. We should remove that check as well.
Backporting the indexing and ipni tasks from mkv2 to the pdpv0 branch.
Code is at least not obviously broken but testing is a work in progress.