Skip to content
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

Deprecate mappintracker and adopt stateless tracker #929

Closed
4 tasks
hsanjuan opened this issue Sep 27, 2019 · 0 comments · Fixed by #944
Closed
4 tasks

Deprecate mappintracker and adopt stateless tracker #929

hsanjuan opened this issue Sep 27, 2019 · 0 comments · Fixed by #944
Assignees
Labels
exp/intermediate Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature P1 High: Likely tackled by core team if no one steps up

Comments

@hsanjuan
Copy link
Collaborator

hsanjuan commented Sep 27, 2019

The mappintracker component has been used since the very early days.

It keeps an in-memory representation of the pinset along with the current PinStatus for each CID. This means that the actual status as seen by Cluster for a CID (pinned) may differ of that actual status in IPFS (unpinned). This also means that the map needs to be filled-in during cluster boot.

To avoid these problems, the main component regularly runs a StatusSync. The first time it runs it will trigger a Track operation for each CID which results in a pin/ls operation on IPFS for each CID which is additionally heavy.

The stateless pin tracker was developed to avoid all this. Rather than keeping all the information in memory, it would be fetched from IPFS when needed. If we need to check if a pin ls pinned, we do pin/ls then.

Additionally, most getting-out-of-sync issues were solved so that essentially does not happen and the sync operation is very redundant at this point (for both trackers).

On the downside, the stateless pin tracker uses RPC calls to access the state, which is super innefficient and effectively loads the pinset onto memory as well by serializing and deserializing it.

Let's fix all this.

Actionables

  • Remove the mappintracker (module, tests, service flags)
  • Remove the Sync operation from everywhere (ctl, api, pintrackers)
  • Modify the stateless pintracker to use a state.ReadOnly so operations like localStatus can use that rather than calling RPC methods. The main problem here is that this is provided by the State methods in consensus, and this method can only be called when consensus is ready. Thus it will need to be provided in a delayed fashion (SetState method called appropiately from the Cluster main component (?))
  • Ensure things are working like before. Overall this should simplify and remove more code than add. However sometimes some tests in stateless tracker fail. It is very important to understand how the optracker module works and how it is used to handle the pinning operation lifetime in scenarios with concurrency (very tricky). The StateSync operation in cluster.go will become a private method and should lose most of its logic (it needs to keep things like checking for pin expiration), probably it should have a different name.
@hsanjuan hsanjuan added kind/enhancement A net-new feature or improvement to an existing feature exp/intermediate Prior experience is likely helpful P1 High: Likely tackled by core team if no one steps up labels Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/intermediate Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature P1 High: Likely tackled by core team if no one steps up
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants