-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement the core federation protocol #2491
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2491 +/- ##
==========================================
- Coverage 47.02% 46.88% -0.14%
==========================================
Files 85 90 +5
Lines 10718 11242 +524
==========================================
+ Hits 5040 5271 +231
- Misses 5079 5337 +258
- Partials 599 634 +35 ☔ View full report in Codecov by Sentry. |
}, nil | ||
} | ||
|
||
func (f *Federation) Start(_ context.Context) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why take and not use cancel here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it might be surprising to the user that start context is tied to the entire lifecycle of federation rather than just starting it.
link ipld.LinkPrototype | ||
} | ||
|
||
//go:embed schema.ipldsch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an interesting pattern - does it impact startup time to do this on startup rather than a compile time step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not measured it. I also have not heard complains about its current usage elsewhere being slow.
Happy to measure it if you think it's a blocker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few minor coding issues commented on, but the more important thing is to establish an understanding of how to do:
- Discovery of new indexers within the federation.
- Authentication to allow membership
- Reconsiliation process.
As for discovery, an indexer could be given a list of other indexer peer IDs. Or, indexers could periodically announce their presence on a libp2p topic, where the topic is the name of the federation. Other indexers subscribed to that same topic/federation whould choose whether to accept another indexer as a federation peer.
If discovery is limited to a configured list, where is this list published? If using libp2p pubsub, is there some bootstrap node somewhere? Is there a registry of "trusted" indexers for a given federation?
Epoch uint64 | ||
VectorClock uint64 | ||
Providers ProvidersIngestStatusMap | ||
Previous ipld.Link |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what, if any, situations will a previous snapshot be asked for by another indexer? A previous snapshot does not seem necessary for reconciliation.
The only situation I can see needing a previous snapshot for, is when an ipni instance (ipni-A) sees that another instance (ipni-B) has a provider unknown to ipni-A. Then ipni-A might look at previous snapshots to see if it used to know about the provider which was later delisted, or whether the provider is new to ipni-A.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A previous snapshot does not seem necessary for reconciliation.
Right.
The rationale for having a chain of snapshot is to provide opportunity for reputation measurements in the future since IPNI will need some sort of reputation system atop a permissionless federation setup.
Side note: It might be worth bringing up this question on the spec PR as it relates more to protocol design itself than the implementation. I think it is worth writing a rationale for its existence in the spec at least, if we reach the consensus to keep it.
func (f *Federation) reconcile(ctx context.Context, t time.Time) error { | ||
// TODO: For each f.peers, get head, get snapshot, f.vc.reconcile, aggregate providers. | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given snapshots:
snap-ipni-A (head)
--------------------
Provider-P1 lastAd=10
Provider-P2 lastAd=5
Prev ---+
|
--------------------
Provider-P1 lastAd=3
Provider-P2 lastAd=4
snap-ipni-B (head)
--------------------
Provider-P1 lastAd=6
Provider-P2 lastAd=11
Prev ---+
|
--------------------
Provider-P1 lastAd=4
Provider-P2 lastAd=3
I think the algorithm works something like:
ipni-B reconciles P1:
- Get head ad for P1 (15)
- If no vector clock difference for P1
- Traverse P1 ad chain until ipni-A-P1-lastAd(10) or ipni-B-P1-lastAd(6)
- Reconcile vector clocks for P1 and compute difference
- If vector clock difference for P1 known:
- Determine if ipni-A is ahead in indexing of P2
- See that ipni-A is indexed farther, so sync P1 using ipni-A up to ad 10
ipni-B reconciles P2:
- Get head ad for P2 (12)
- If no vector clock difference for P2
- Traverse P2 ad chain until ipni-A-P2-lastAd(11) or ipni-B-P1-lastAd(5)
- Reconcile vector clocks for P2 and compute difference
- If vector clock difference for P2 known:
- Determine if ipni-A is ahead in indexing of P2
- See that ipni-B is indexed farther, nothing to do
NOTE:
- Previous snapshots are not examined in these situations. Maybe only when a provider is known to one ipni and not the other. Is this right?
- The vector clock is only an optimization, and can only be used once an initial difference is established. I assume the clocks always accumulate and do not reset in each snapshot. Correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Right.
- Correct. Vector clocks are monotonically increasing.
|
First pass at implementing the IPNI federation protocol: * core data structures/functionality * periodic snapshot * basic vector clock reconciliation * HTTP serve mux for `/ipni/v1/fed`
8ca9196
to
b049a7c
Compare
mux.HandleFunc("/ipni/v1/fed/head", f.handleV1FedHead) | ||
mux.HandleFunc("/ipni/v1/fed/", f.handleV1FedSubtree) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want this to be just /fed
and then mount it within the /ipni/v1
general mux instead of parallel to it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I strongly advise to have a unified prefix (plus version) for IPNI URIs. I think it makes it clear what the grouping is as a coherent protocol for content discovery.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re mounting, the ingress reverse proxy would do the rewrite and it really makes no difference what is here. Except for the fact that the current mux makes it possible to run a fully self contained federation server separate from an indexer itself.
Remove untick from vector clock at the risk of having increments larger than one for the local vector clock in edge cases where persistence of snapshot may fail in the midst of a cycle. This is an acceptable risk considering the difficulty in implementing transactional IPLD node storage and the application of it in this particular case.
dcd2af8
to
1e711a3
Compare
It is still useful to have a head with no link to learn for example a federation member's public key or topic.
First pass at implementing the IPNI federation protocol:
/ipni/v1/fed
See: ipni/specs#27