State: Use go-datastore to implement the state interface #655

hsanjuan · 2019-01-25T12:16:17Z

Since the beginning, we have used a Go map to store the shared state (pinset)
in memory. The mapstate knew how to serialize itself so that libp2p-raft would
know how to write to disk when it:

Saved snapshots of the state on shutdown
Sent the state to a newcomer peer

hashicorp.Raft assumes an in-memory state which is snapshotted from time to
time and read from disk on boot.

This commit adds a dsstate implementation of the state interface using
go-datastore. This allows to effortlessly switch to a disk-backed state in
the future (as we will need), and also have at our disposal the different
implementations and utilities of Datastore for fine-tuning (caching, batching
etc.).

mapstate has been reworked to use dsstate. Ideally, we would not even need
mapstate, as it would suffice to initialize dsstate with a
MapDatastore. BUT, we still need it separate to be able to auto-migrate to
the new format.

This will be the last migration with the current system. Once this has been
released and users have been able to upgrade we will just remove mapstate as
it is now.

License: MIT
Signed-off-by: Hector Sanjuan code@hector.link

Depends on libp2p/go-libp2p-raft#50

hsanjuan · 2019-01-25T12:18:05Z

Also, kind of depends on discussion #654 . If we slightly changed how things are serialized soon, I'd rather not have to migrate right after this.

lanzafame · 2019-01-31T04:33:06Z

LGTM. Only thing would be a better explanation of what the migration process is and how the migration process will be different in the future, i.e. post v6.

lanzafame · 2019-01-31T05:46:02Z

@hsanjuan It seems that in some cases, this PR has slowed down cluster as there have now been 2 test timeout failures. I am happy to bump the timeout up again and then chip away at getting it down again.

coveralls · 2019-01-31T05:54:07Z

Coverage decreased (-0.3%) to 64.888% when pulling 46801aa on feat/datastore into 3059ab3 on master.

hsanjuan · 2019-01-31T18:37:43Z

Also, kind of depends on discussion #654 . If we slightly changed how things are serialized soon, I'd rather not have to migrate right after this.

I don't fully know. But since checking state version will mean opening the datastore and reading one key, we know that will be different. The migration will be looping the keys and overwriting them with their updated form.

The key idea is that Marshal/Unmarshal will either not be used, or can happen normally regardless of the state version (unlike now).

hsanjuan · 2019-01-31T18:41:07Z

@hsanjuan It seems that in some cases, this PR has slowed down cluster as there have now been 2 test timeout failures. I am happy to bump the timeout up again and then chip away at getting it down again.

I only see jenkins, which I retriggered now. It make take a while before I merge this anyway.

Does this approval include changes in go-libp2p-raft ?

lanzafame · 2019-02-01T06:46:55Z

@hsanjuan I just added some comments on the go-libp2p-raft PR.

hsanjuan · 2019-02-14T01:10:56Z

@lanzafame do me a favour and have a look to the last commits (added tracing) and lmk if we can leave this ready for merge, as I want it to go before #670 .

api/pb/types.pb.go

lanzafame · 2019-02-18T01:53:19Z

api/pb/types.pb.go

+// proto package needs to be updated.
+const _ = proto.ProtoPackageIsVersion3 // please upgrade the proto package
+
+type Pin_PinType int32


Was the go package flag used in type.pb so that it generates 'idiomatic' type names?

cluster.go

lanzafame · 2019-02-18T12:33:21Z

api/pb/types.proto

+
+message Pin {
+  bytes Cid = 1;
+  enum PinType {


s/PinType/Type

I can't have an enum called Type and a field called Type

let's leave this as is, I don't mind so much and we can change it later.

lanzafame · 2019-02-18T13:13:46Z

api/types.go

-	ReplicationFactorMax int    `json:"replication_factor_max"`
-	Name                 string `json:"name"`
-	ShardSize            uint64 `json:"shard_size"`
+	ReplicationFactorMin int               `json:"replication_factor_min" codec:"rn,omitempty"`


@hsanjuan Is there a compile-time or test-time check that we can add so that we don't conflict with the codec variable?

As discussed, tests break in most cases when screwing up the codec field names.

Additionally I have added a test that verifies that there are no dup fields.

Since the beginning, we have used a Go map to store the shared state (pinset) in memory. The mapstate knew how to serialize itself so that libp2p-raft would know how to write to disk when it: * Saved snapshots of the state on shutdown * Sent the state to a newcomer peer hashicorp.Raft assumes an in-memory state which is snapshotted from time to time and read from disk on boot. This commit adds a `dsstate` implementation of the state interface using `go-datastore`. This allows to effortlessly switch to a disk-backed state in the future (as we will need), and also have at our disposal the different implementations and utilities of Datastore for fine-tuning (caching, batching etc.). `mapstate` has been reworked to use dsstate. Ideally, we would not even need `mapstate`, as it would suffice to initialize `dsstate` with a `MapDatastore`. BUT, we still need it separate to be able to auto-migrate to the new format. This will be the last migration with the current system. Once this has been released and users have been able to upgrade we will just remove `mapstate` as it is now. License: MIT Signed-off-by: Hector Sanjuan <code@hector.link>

Additionally, remove persisting the state version to the go-datastore. In the future versions of the state, there is not a global format anymore (with a global version). Instead, every pin element can potentially be stored in a different version. License: MIT Signed-off-by: Hector Sanjuan <hector@protocol.ai>

hsanjuan · 2019-02-19T20:01:25Z

@lanzafame I squashed, rebased and cleaned up. Then I tested with larger states and had to fix some issues (latest commit). Can you check again? I think we can merge then.

License: MIT Signed-off-by: Hector Sanjuan <hector@protocol.ai>

lanzafame · 2019-02-20T10:59:23Z

@hsanjuan LGTM

hsanjuan self-assigned this Jan 25, 2019

hsanjuan requested a review from lanzafame January 25, 2019 12:16

ghost added the status/in-progress In progress label Jan 25, 2019

lanzafame previously approved these changes Jan 31, 2019

View reviewed changes

hsanjuan dismissed lanzafame’s stale review via bfdf575 February 7, 2019 23:18

hsanjuan mentioned this pull request Feb 15, 2019

Dependency alignment for libp2p-6.0.33, dag libraries and others #675

Merged

lanzafame suggested changes Feb 18, 2019

View reviewed changes

lanzafame reviewed Feb 18, 2019

View reviewed changes

ipfs-cluster deleted a comment from GitCop Feb 19, 2019

hsanjuan force-pushed the feat/datastore branch from 81c220e to f78ca81 Compare February 19, 2019 18:01

hsanjuan force-pushed the feat/datastore branch from f78ca81 to d57b814 Compare February 19, 2019 18:31

Set version for mapstate

46801aa

License: MIT Signed-off-by: Hector Sanjuan <hector@protocol.ai>

hsanjuan mentioned this pull request Feb 19, 2019

Support PinPath, UnpinPath (resolve before pinning) #634

Merged

2 tasks

lanzafame approved these changes Feb 20, 2019

View reviewed changes

hsanjuan merged commit f57c5e4 into master Feb 20, 2019

ghost removed the status/in-progress In progress label Feb 20, 2019

hsanjuan deleted the feat/datastore branch February 20, 2019 11:20

hsanjuan mentioned this pull request Feb 20, 2019

Release 0.10.0 #678

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

State: Use go-datastore to implement the state interface #655

State: Use go-datastore to implement the state interface #655

hsanjuan commented Jan 25, 2019

hsanjuan commented Jan 25, 2019

lanzafame commented Jan 31, 2019

lanzafame commented Jan 31, 2019

coveralls commented Jan 31, 2019 •

edited

hsanjuan commented Jan 31, 2019

hsanjuan commented Jan 31, 2019

lanzafame commented Feb 1, 2019

hsanjuan commented Feb 14, 2019

lanzafame Feb 18, 2019

lanzafame Feb 18, 2019

hsanjuan Feb 19, 2019

hsanjuan Feb 19, 2019

lanzafame Feb 18, 2019

hsanjuan Feb 19, 2019

hsanjuan Feb 19, 2019

hsanjuan commented Feb 19, 2019

lanzafame commented Feb 20, 2019

State: Use go-datastore to implement the state interface #655

State: Use go-datastore to implement the state interface #655

Conversation

hsanjuan commented Jan 25, 2019

hsanjuan commented Jan 25, 2019

lanzafame commented Jan 31, 2019

lanzafame commented Jan 31, 2019

coveralls commented Jan 31, 2019 • edited

hsanjuan commented Jan 31, 2019

hsanjuan commented Jan 31, 2019

lanzafame commented Feb 1, 2019

hsanjuan commented Feb 14, 2019

lanzafame Feb 18, 2019

Choose a reason for hiding this comment

lanzafame Feb 18, 2019

Choose a reason for hiding this comment

hsanjuan Feb 19, 2019

Choose a reason for hiding this comment

hsanjuan Feb 19, 2019

Choose a reason for hiding this comment

lanzafame Feb 18, 2019

Choose a reason for hiding this comment

hsanjuan Feb 19, 2019

Choose a reason for hiding this comment

hsanjuan Feb 19, 2019

Choose a reason for hiding this comment

hsanjuan commented Feb 19, 2019

lanzafame commented Feb 20, 2019

coveralls commented Jan 31, 2019 •

edited