Announce Bitswap records to indexer #839
Conversation
hannahhoward
force-pushed
the
feat/announce-to-indexer
branch
2 times, most recently
from
3afabbb
to
a211e4e
Compare
| lc.Append(fx.Hook{ | ||
| OnStart: func(ctx context.Context) error { | ||
| go func() { | ||
| _ = w.IndexerAnnounceAllDeals(ctx) |
There was a problem hiding this comment.
Looking at how IndexerAnnounceAllDeals is implemented it looks like it will announce
- all legacy deals that are in a sealing state (but not in an error or completed state)
- all new deals that have not already been announced (and are not in an error or completed state)
Is that what we want to happen here?
Also keep in mind that some providers have up to 100k deals.
There was a problem hiding this comment.
If there is no change to the protocols supported, this is essentially a no-op -- the call to announce will error with ErrAlreadyPublished.
per @willscott at the moment there is no more efficient way to do this.
There was a problem hiding this comment.
Ok but I don't think this is going to re-announce deals that are not in one of the two categories I mentioned. For example non-legacy deals that have already been announced.
There was a problem hiding this comment.
ah I now see what you mean.
There was a problem hiding this comment.
ok I now have more questions having read the code. I'm writing it up in a new issue. Basically, based on the behavior of the code as it exists, I really can't figure out what IndexerAnnounceAllDeals was intended to do.
The intent of the go-fil-markets code looks like a re-announce, while the code in boost looks like a first time announce, which will include several deals that really shouldn't be announced yet (i.e. haven't even had a data transfer)
Will document in more detail in a seperate issue.
There was a problem hiding this comment.
ok @dirkmc I think I've put this in a reasonable state. There are two new commits --
filecoin-project/go-fil-markets@5dd7bc1
- this removes StorageDealStaged from the list of states that announce, because StorageDealStaged is the stage where they get announces (vaguely equivalent to dealcheckpoints.AddedPiece in Boost)
- otherwise, I think the go-fil-markets logic is correct --
- StorageDealActive describes the entire time the deal is on chain, which seems like a reasonable bound keep the announcement alive, and it is already among the states that get reannounced. The actual "completion states" are either errors or after the deal has actually expired (i.e. we're past SectorEndEpoch).
- this changes the condition for announce to essentially be only those deals that are at the IndexedAndAnnounced. Technically, it filters out all that are < IndexedAndAnnouced and >= Complete, which is currently the same as != IndexedAndAnnounced but I thought it'd be useful to treat this as a range in case we add something between IndexedAndAnnounced and Complete in the future.
There was a problem hiding this comment.
this addresses most of the issues in #882 though not all of them. It certainly shouldn't make things worse. :)
support announcing on bitswap when configured to do so
hannahhoward
force-pushed
the
feat/announce-to-indexer
branch
from
a211e4e
to
9cd1540
Compare
|
I'm not sure why test-all is failing -- I don't seem to get this failure locally and it seems unrelated. |
|
LGTM 👍 @hannahhoward thanks for digging through all this code and figuring out how to fix it the right way 🙏 |
Goals
When booster-bitswap is configured, announce Bitswap as a supported protocol to the indexer
Implementation