[DO NOT MERGE] Pushsync pss by nolash · Pull Request #1768 · ethersphere/swarm

nolash · 2019-09-17T16:13:34Z

I push the current state. It implements async message handling in pss to address the performance issues raised by @nonsense as pointed out by @janos
I do not want to sit on this too long, instead commit a version with broken tests in pss which i skipped with FIXME!. @nolash @jpeletier would you like to jump in and help fix this. Also a rebase is needed and there will be pss conflicts due to the pssMsg PR just merged.

the failing tests are:

--- FAIL: TestAddressMatchProx (0.00s)
--- FAIL: TestRawAllow (0.00s)

Please feel free to modify the pss changes to your liking.

zelig · 2019-09-17T20:14:21Z

+	}
+
+	tSynced := tag.Get(chunk.StateSynced)
+	if tStored != stored {


ooops, should be synced

zelig · 2019-09-17T20:18:58Z

 		hp := network.NewHiveParams()
 		hp.KeepAliveInterval = time.Duration(200) * time.Millisecond
 		hp.Discovery = false
+		hp.DisableAutoConnect = true


this is incorrect here, but All snapshot tests need to set this to true. So this param needs to be passed down in node config.

zelig · 2019-09-17T20:23:55Z

+func (p *Pss) handle(ctx context.Context, peer *protocols.Peer, msg interface{}) error {
+	go func() {
+		pssmsg, ok := msg.(*message.Message)
+		if !ok {


this is not necessary, p2p/protocols package guarantees correct type, and if we added another message the handler should match on type anyway

zelig · 2019-09-17T20:24:31Z

 // note that in these tests we use the raw capability on handlers for convenience
 func TestAddressMatchProx(t *testing.T) {
-
+	t.Skip("FIXME!")


new failing test due to async handling

zelig · 2019-09-17T20:25:23Z

 // verifies that message handlers for raw messages only are invoked when minimum one handler for the topic exists in which raw messages are explicitly allowed
 func TestRawAllow(t *testing.T) {
-
+	t.Skip("FIXME!")


new failing test due to async handling

zelig · 2019-09-17T20:44:26Z

 	// check the tag was created successfully
 	tag := srv.Tags.All()[0]
-	chunktesting.CheckTag(t, tag, 9, 9, 0, 9)
+	chunktesting.CheckTag(t, tag, 8, 8, 0, 0, 0, 8)


we should check why the modified use of context in http/server results in different number of tags. a manifest is missing maybe?

zelig · 2019-09-17T20:44:47Z

 	// check the tag was created successfully
 	tag := srv.Tags.All()[0]
-	chunktesting.CheckTag(t, tag, 9, 9, 0, 9)
+	chunktesting.CheckTag(t, tag, 8, 8, 0, 0, 0, 8)


we should check why the modified use of context in http/server results in different number of tags. a manifest is missing maybe?
@zelig

zelig · 2019-09-17T20:46:03Z

 	// check that the tag was written correctly
 	tag := srv.Tags.All()[0]
-	chunktesting.CheckTag(t, tag, 4, 4, 0, 4)
+	chunktesting.CheckTag(t, tag, 3, 3, 0, 0, 0, 3)


we should check why the modified use of context in http/server results in different number of tags. a manifest is missing maybe?

zelig · 2019-09-17T20:48:37Z

 		return 0, errors.New("address length must match")
 	}
-	return ProxCmp(a, x, y), nil
+	return ProxCmp(a, y, x), nil


this did not conform to its own comment

zelig · 2019-09-17T20:55:27Z

+}
+
+// newServiceFunc constructs a minimal service needed for a simulation test for Push Sync, namely:
+// localstore, netstore, stream and pss


stream -> retrieval
and also bzz needed

janos · 2019-09-19T08:46:57Z

@nolash could you resolve conflicts in pss/pss.go?

…ion wait delay

- storer needs to take netstore not localstore to put the chunk so that fetchers created earlier could respond

storage/pushsync: update NetStore api from master storage/pushsync: add sleeps and a bit more tracing, change subscription wait delay network: try sending receipt only if there is no closer peer storage/pushsync: use netstore, rather than localstore storage/netstore: logs with node id storage/pushsync: opentracing storage/pushsync: propagate origin on receipts is push synced - smoke test network: kademlia closer peer storage: very high retry timeout measure send chunk rlp timer pss timers and goroutine sendChunk traces for chunsk in localstore and subscribepush try minbinsize: 3 increase search timeout ; move tracing before nil check link netstore and delivery rename tag New to tag Create swarm-smoke: fix check max prox hosts for pull/push sync modes tag roundtrip wip more logs kad as part of pusher? kad to storer; revert pss change metrics to pss outbox len metric emit metrics once every 10sec. pss: refactor storage/pushsync: closer than me trace chunk: adding XOR comparison disable pushsync tests, as they are not setting up kad properly

- WaitTillDone does check initially - introduce IncN to increment with n - api: inspector to use tag.Done - unexport context, span, and Context() to be used by http server

- minor improvements in logging - calls on Kademlia directly on Pss struct - add IsClosestTo function to pubsub using kademlia.IsClosestTo

- move package from storage to toplevel - only closest peer to address returns a receipt - IsClosestTo(addr) is now part of the PubSub interface - rename TestPushSyncAndStoreWithLoopback to TestProtocol - for TestPusher and , IsClosest is mocked properly - remove Origin field from receipt message - pushed item remembers first and last sent time - retryInterval is dynamically set as 2 * average roundtrip (excluding outliers) - early send check is removed from unit test - receipts channel is just []byte for address - we are using mutli-set for setting synced status on chunks - tag increment now follows multiset and not incorrectly when the receipt arrives for this remember syncedItems is needed alongside syncedAddrs - in pusher sync loop, we use static context - correctly unsubscribe DB.SubscribePush - if pusher is closest node to a chunk, chunk is not sent using pubsub, but receipt is directly sent to self using a shortcut - therefore no self send is needed in storer - loopBack pubsub is shared in the protocol unit test but wrapped in different testPubSub structs to control the behaviour of IsClosestTo function - added delayResponse to TestProtocol too - testPushSyncIndex SubscribePush now increments tag StateStored correctly only first time - fix checkTags to wait for synced status (it caused occasional flakiness) - extract simulation test parameters (nodes/chunks/testcases) as command line flags, default is 16/16/16

- introduce custom logger on pusher/storer - dynamic setting of retryinterval extracted - timeout changed back - fix and use testutil.checkTags - use tag context - tags.Create and NewTag has context argument - fix pusher.Close before localstore

- changed order of select cases - have Tests on top followed by helpers - add sent,synced check to testutil.CheckTag - add timeout to close wait - adapt to testutil.Init() - length protection to label helper - resolve rebase conflicts

- simplify simulation using error group - make pss message handling asynchronous - simulation NewBzzInProc hive disable autoconnect (to check) - pss outbox go routine leak

nolash · 2019-09-20T10:47:00Z

Continued in #1782

zelig added do-not-merge push-sync labels Sep 17, 2019

zelig reviewed Sep 17, 2019

View reviewed changes

nonsense and others added 26 commits September 19, 2019 12:08

swarm: push sync

7a665c9

storage/pushsync: update NetStore api from master

4fc27ae

storage/pushsync: add sleeps and a bit more tracing, change subscript…

2665b9a

…ion wait delay

chunk; fix "already exists" error tags.Uid no need for rng obj

b0c8dd1

storage/pushsync: fix storer initialisation

4e9f090

- storer needs to take netstore not localstore to put the chunk so that fetchers created earlier could respond

chunk: tags improvement

4cc7016

- WaitTillDone does check initially - introduce IncN to increment with n - api: inspector to use tag.Done - unexport context, span, and Context() to be used by http server

shed: linting fix

c6c9a86

pot: DistanceCmp opposite of ProxCmp reflecting doc

9ffe9b1

network: add IsClosestTo(addr, filter) using pot.DistanceCmp

70c9d5d

pss: fix hashpool init to use keccak256

dc90c66

- minor improvements in logging - calls on Kademlia directly on Pss struct - add IsClosestTo function to pubsub using kademlia.IsClosestTo

pushsync: simulation test params for appveyor

98263cb

pushsync: asynchronous send in sync forever loop; disabled ordering test

b2e077c

api/http: remove periodicTagTrace

51f3676

pushsync: close item span at end of roundtrip

f634c9e

pushsync: amend pusher test, remove loopback sync option

536ec86

pushsync: if no new items set timer to half a second

49db062

pushsync: address PR review feedback

4023b04

pushsync, testutil: address review comments

d879022

- changed order of select cases - have Tests on top followed by helpers - add sent,synced check to testutil.CheckTag - add timeout to close wait - adapt to testutil.Init() - length protection to label helper - resolve rebase conflicts

removed accidentally added files

9e18234

pushsync, pss: adapt simulation to new retrieval

b6788c8

- simplify simulation using error group - make pss message handling asynchronous - simulation NewBzzInProc hive disable autoconnect (to check) - pss outbox go routine leak

pss: Rebase cleanup

59a1267

pushsync: Correct call for retrieve

0f60551

pss: Rehabilitate tests failing due to async handler

a5dd509

nolash force-pushed the pushsync-pss-2 branch from 9c9b33d to a5dd509 Compare September 19, 2019 10:09

pushsync: Correct bzzaddr constructor call

090bf86

janos mentioned this pull request Sep 19, 2019

pushsync with async pss #1782

Merged

nolash closed this Sep 20, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DO NOT MERGE] Pushsync pss#1768

[DO NOT MERGE] Pushsync pss#1768
nolash wants to merge 27 commits intoethersphere:masterfrom
nolash:pushsync-pss-2

nolash commented Sep 17, 2019 •

edited by zelig

Loading

Uh oh!

zelig Sep 17, 2019

Uh oh!

zelig Sep 17, 2019

Uh oh!

zelig Sep 17, 2019

Uh oh!

zelig Sep 17, 2019

Uh oh!

zelig Sep 17, 2019

Uh oh!

zelig Sep 17, 2019

Uh oh!

zelig Sep 17, 2019

Uh oh!

zelig Sep 17, 2019

Uh oh!

zelig Sep 17, 2019

Uh oh!

zelig Sep 17, 2019

Uh oh!

janos commented Sep 19, 2019

Uh oh!

nolash commented Sep 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

nolash commented Sep 17, 2019 • edited by zelig Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janos commented Sep 19, 2019

Uh oh!

nolash commented Sep 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nolash commented Sep 17, 2019 •

edited by zelig

Loading