Serialize chain elements #583

Bolodya1997 · 2020-11-11T06:56:21Z

Issue

Request, Close events for the same Connection.ID should be processed serially in the whole network service chain. At the moment, there is no such convention and so every new chain element should use some internal synchronization to make such thing inside of it. It leads to the multiple times serializing already serialized events but we still need to make such things for all newly created chain elements because there is no guarantee that someone would do this instead of us.

Solution

Create single chain element that is responsive for all per-connection events serializing.

pkg/networkservice/common/serialize/cancellable_executor.go

pkg/networkservice/common/serialize/README.md

edwarnicke · 2020-11-18T13:50:44Z

Question: Why are you serializing Close and Request separately for a given Connection Id? Its not what I expected, and you usually have good reasons for such things :)

edwarnicke · 2020-11-18T13:51:25Z

Also, you may find this potentially useful:

#590

Bolodya1997 · 2020-11-18T15:10:44Z

Question: Why are you serializing Close and Request separately for a given Connection Id? Its not what I expected, and you usually have good reasons for such things :)

They are not being serialized separately - there are 2 different wrappers for the same executor, because after the Close event we should mark executor as closed and we can't access executor.state from some chain element other than serialize.

edwarnicke · 2020-11-18T17:36:59Z

Question: Why are you serializing Close and Request separately for a given Connection Id? Its not what I expected, and you usually have good reasons for such things :)

They are not being serialized separately - there are 2 different wrappers for the same executor, because after the Close event we should mark executor as closed and we can't access executor.state from some chain element other than serialize.

Got it :)

denis-tingaikin · 2020-11-23T11:38:24Z

@xzfc Could you take a look at this PR? It seems to me this can be usful for refresh

pkg/networkservice/common/serialize/serializer.go

pkg/networkservice/common/timeout/README.md

pkg/networkservice/common/serialize/context.go

denis-tingaikin

This version looks better. Added comments.

pkg/tools/multiexecutor/multi_executor.go

pkg/networkservice/common/serialize/executor_func.go

pkg/networkservice/common/serialize/context.go

pkg/tools/multiexecutor/multi_executor_test.go

denis-tingaikin · 2020-11-26T16:16:35Z

pkg/networkservice/common/timeout/server_test.go

@@ -141,7 +143,7 @@ func stressTestRequest() *networkservice.NetworkServiceRequest {
 	}
 }

-func TestTimeoutServer_StressTest_DoubleClose(t *testing.T) {
+func TestTimeoutServer_StressTest(t *testing.T) {


How long is this test running? Note: we should expect 1s for the unit test.

20ms locally.

build and test (ubuntu-latest)

ok github.com/networkservicemesh/sdk/pkg/networkservice/common/serialize 0.896s

build and test (macos-latest)

ok github.com/networkservicemesh/sdk/pkg/networkservice/common/serialize 0.760s

build and test (windows-latest) <-- it is actually very slow for all tests

ok github.com/networkservicemesh/sdk/pkg/networkservice/common/serialize 1.813s

denis-tingaikin

LGTM

Signed-off-by: Vladimir Popov <[email protected]>

denis-tingaikin · 2020-12-01T09:16:58Z

pkg/networkservice/common/serialize/context.go

+type contextKeyType string
+
+// IExecutor is a serialize.Executor interface type
+type IExecutor interface {


Please lets follow go interface naming convention https://golang.org/doc/effective_go.html#interface-names

Renamed:

IExecutor -> Executor .Executor() -> .GetExecutor()

Signed-off-by: Vladimir Popov <[email protected]>

denis-tingaikin

Merging...

@edwarnicke Let us know if you have something in mind related to these changes.

…k@master networkservicemesh/sdk#583 networkservicemesh/sdk PR link: networkservicemesh/sdk#583 networkservicemesh/sdk commit message: commit 98deb86e4ee85abd4622882171b79433be93c335 Author: Vladimir Popov <[email protected]> Date: Tue Dec 1 18:10:53 2020 +0700 Serialize chain elements (#583) * Add serialize Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with serialize Signed-off-by: Vladimir Popov <[email protected]> * Rework serialize with RequestExecutor, CloseExecutor Signed-off-by: Vladimir Popov <[email protected]> * Fix race condition issue in serialize Signed-off-by: Vladimir Popov <[email protected]> * Update github.com/edwarnicke/serialize version Signed-off-by: Vladimir Popov <[email protected]> * Rework serialize Signed-off-by: Vladimir Popov <[email protected]> * Fix URL in README Signed-off-by: Vladimir Popov <[email protected]> * Use atomic.AddUint32() instead of uuid.New().Id() Signed-off-by: Vladimir Popov <[email protected]> * Add multiexecutor Signed-off-by: Vladimir Popov <[email protected]> * Rework serialize with multi executor Signed-off-by: Vladimir Popov <[email protected]> * Move multi executor to serialize Signed-off-by: Vladimir Popov <[email protected]> * Rename `IExecutor` to `Executor` Signed-off-by: Vladimir Popov <[email protected]> Signed-off-by: NSMBot <[email protected]>

* Add serialize Signed-off-by: Vladimir Popov <[email protected]> * Rework timeout with serialize Signed-off-by: Vladimir Popov <[email protected]> * Rework serialize with RequestExecutor, CloseExecutor Signed-off-by: Vladimir Popov <[email protected]> * Fix race condition issue in serialize Signed-off-by: Vladimir Popov <[email protected]> * Update github.com/edwarnicke/serialize version Signed-off-by: Vladimir Popov <[email protected]> * Rework serialize Signed-off-by: Vladimir Popov <[email protected]> * Fix URL in README Signed-off-by: Vladimir Popov <[email protected]> * Use atomic.AddUint32() instead of uuid.New().Id() Signed-off-by: Vladimir Popov <[email protected]> * Add multiexecutor Signed-off-by: Vladimir Popov <[email protected]> * Rework serialize with multi executor Signed-off-by: Vladimir Popov <[email protected]> * Move multi executor to serialize Signed-off-by: Vladimir Popov <[email protected]> * Rename `IExecutor` to `Executor` Signed-off-by: Vladimir Popov <[email protected]> Signed-off-by: Sergey Ershov <[email protected]>

Bolodya1997 force-pushed the serialize branch from b8d6686 to a77303e Compare November 11, 2020 07:07

denis-tingaikin reviewed Nov 13, 2020

View reviewed changes

pkg/networkservice/common/serialize/cancellable_executor.go Outdated Show resolved Hide resolved

Bolodya1997 force-pushed the serialize branch from a77303e to c8f5457 Compare November 16, 2020 03:54

Bolodya1997 marked this pull request as draft November 16, 2020 06:48

Bolodya1997 force-pushed the serialize branch from c8f5457 to d4ef042 Compare November 16, 2020 06:53

Bolodya1997 marked this pull request as ready for review November 16, 2020 06:53

Bolodya1997 force-pushed the serialize branch 2 times, most recently from 570d15d to b19c7ef Compare November 16, 2020 12:34

denis-tingaikin requested a review from xzfc November 16, 2020 12:56

xzfc reviewed Nov 16, 2020

View reviewed changes

pkg/networkservice/common/serialize/README.md Outdated Show resolved Hide resolved

Bolodya1997 force-pushed the serialize branch from b19c7ef to bd6fe65 Compare November 17, 2020 03:22

edwarnicke reviewed Nov 18, 2020

View reviewed changes

pkg/networkservice/common/serialize/README.md Outdated Show resolved Hide resolved

Bolodya1997 force-pushed the serialize branch from bd6fe65 to 5583234 Compare November 23, 2020 11:34

denis-tingaikin requested a review from xzfc November 23, 2020 11:35

xzfc reviewed Nov 23, 2020

View reviewed changes

pkg/networkservice/common/serialize/serializer.go Outdated Show resolved Hide resolved

xzfc reviewed Nov 23, 2020

View reviewed changes

pkg/networkservice/common/timeout/README.md Show resolved Hide resolved

Bolodya1997 force-pushed the serialize branch 2 times, most recently from 09b73a0 to 76ac337 Compare November 24, 2020 08:15

denis-tingaikin reviewed Nov 25, 2020

View reviewed changes

pkg/networkservice/common/serialize/context.go Outdated Show resolved Hide resolved

denis-tingaikin requested changes Nov 26, 2020

View reviewed changes

Bolodya1997 marked this pull request as draft November 27, 2020 09:29

Bolodya1997 force-pushed the serialize branch from df6242b to 08cebc5 Compare December 1, 2020 04:44

Bolodya1997 marked this pull request as ready for review December 1, 2020 04:46

denis-tingaikin approved these changes Dec 1, 2020

View reviewed changes

Add serialize

d0c26f5

Signed-off-by: Vladimir Popov <[email protected]>

Vladimir Popov added 3 commits December 1, 2020 15:21

Use atomic.AddUint32() instead of uuid.New().Id()

8238a71

Signed-off-by: Vladimir Popov <[email protected]>

Add multiexecutor

8cb3305

Signed-off-by: Vladimir Popov <[email protected]>

Rework serialize with multi executor

406d0c0

Signed-off-by: Vladimir Popov <[email protected]>

Bolodya1997 force-pushed the serialize branch from 08cebc5 to 507f2e9 Compare December 1, 2020 08:23

Move multi executor to serialize

f8a5f54

Signed-off-by: Vladimir Popov <[email protected]>

Bolodya1997 force-pushed the serialize branch from 507f2e9 to f8a5f54 Compare December 1, 2020 08:35

denis-tingaikin reviewed Dec 1, 2020

View reviewed changes

Rename IExecutor to Executor

3978cab

Signed-off-by: Vladimir Popov <[email protected]>

Bolodya1997 force-pushed the serialize branch from d7c9aba to 3978cab Compare December 1, 2020 10:28

denis-tingaikin reviewed Dec 1, 2020

View reviewed changes

denis-tingaikin merged commit 98deb86 into networkservicemesh:master Dec 1, 2020

Bolodya1997 deleted the serialize branch January 20, 2021 06:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Serialize chain elements #583

Serialize chain elements #583

Bolodya1997 commented Nov 11, 2020

edwarnicke commented Nov 18, 2020

edwarnicke commented Nov 18, 2020

Bolodya1997 commented Nov 18, 2020

edwarnicke commented Nov 18, 2020

denis-tingaikin commented Nov 23, 2020

denis-tingaikin left a comment

denis-tingaikin Nov 26, 2020

Bolodya1997 Dec 1, 2020

Bolodya1997 Dec 1, 2020

denis-tingaikin left a comment

denis-tingaikin Dec 1, 2020 •

edited

Loading

Bolodya1997 Dec 1, 2020

denis-tingaikin left a comment

Serialize chain elements #583

Serialize chain elements #583

Conversation

Bolodya1997 commented Nov 11, 2020

Issue

Solution

edwarnicke commented Nov 18, 2020

edwarnicke commented Nov 18, 2020

Bolodya1997 commented Nov 18, 2020

edwarnicke commented Nov 18, 2020

denis-tingaikin commented Nov 23, 2020

denis-tingaikin left a comment

Choose a reason for hiding this comment

denis-tingaikin Nov 26, 2020

Choose a reason for hiding this comment

Bolodya1997 Dec 1, 2020

Choose a reason for hiding this comment

Bolodya1997 Dec 1, 2020

Choose a reason for hiding this comment

denis-tingaikin left a comment

Choose a reason for hiding this comment

denis-tingaikin Dec 1, 2020 • edited Loading

Choose a reason for hiding this comment

Bolodya1997 Dec 1, 2020

Choose a reason for hiding this comment

denis-tingaikin left a comment

Choose a reason for hiding this comment

denis-tingaikin Dec 1, 2020 •

edited

Loading