From 6345fa40441a5e9fd75829487b14f353700255c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E5=BF=97=E5=AE=87?= Date: Thu, 13 Jun 2019 18:17:21 +0800 Subject: [PATCH 1/4] Added testing.md --- pkg/dmsg/testing.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/dmsg/testing.md b/pkg/dmsg/testing.md index 58f0a6848c..d1958a248f 100644 --- a/pkg/dmsg/testing.md +++ b/pkg/dmsg/testing.md @@ -45,6 +45,11 @@ Note that even though `messaging-discovery` is also considered to be an entity o - Both clients will automatically reconnect to the server. - Transports can be established between clientA and clientB. +**`server_disconnect_should_close_transports`** + +- Given: + - + ### Fuzz testing We should test the robustness of the system under different conditions and random order of events. These tests should be written consisiting of x-number of servers, clients and a single discovery. From 60e75713f9078b3e39079fb6412b7f905f472521 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E5=BF=97=E5=AE=87?= Date: Thu, 13 Jun 2019 18:18:33 +0800 Subject: [PATCH 2/4] Updated testing.md --- pkg/dmsg/testing.md | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/pkg/dmsg/testing.md b/pkg/dmsg/testing.md index d1958a248f..16da878971 100644 --- a/pkg/dmsg/testing.md +++ b/pkg/dmsg/testing.md @@ -10,8 +10,6 @@ The individual entities of `dmsg` (`dmsg.Client` and `dmsg.Server`), should be c Note that even though `messaging-discovery` is also considered to be an entity of `dmsg`, however it's mechanics are simple and will not be tested here. -#### Ensure that `Client.Serve()` does not hang - **`failed_accepts_should_not_result_in_hang`** - Given: @@ -33,8 +31,6 @@ Note that even though `messaging-discovery` is also considered to be an entity o - Then: - It should work as expected still. -#### Handling `msg.Server` Failures - **`reconnect_to_server_should_succeed`** - Given: @@ -47,10 +43,10 @@ Note that even though `messaging-discovery` is also considered to be an entity o **`server_disconnect_should_close_transports`** -- Given: - - +- TODO ### Fuzz testing We should test the robustness of the system under different conditions and random order of events. These tests should be written consisiting of x-number of servers, clients and a single discovery. +TODO \ No newline at end of file From 06e061ee14b6b2c41893f247b93a9feff60231c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E5=BF=97=E5=AE=87?= Date: Sat, 15 Jun 2019 23:42:56 +0800 Subject: [PATCH 3/4] Progress on README. --- pkg/dmsg/README.md | 134 ++++++++++++++++++++++++++++ pkg/dmsg/{testing.md => TESTING.md} | 0 pkg/dmsg/frame.go | 10 +-- 3 files changed, 139 insertions(+), 5 deletions(-) create mode 100644 pkg/dmsg/README.md rename pkg/dmsg/{testing.md => TESTING.md} (100%) diff --git a/pkg/dmsg/README.md b/pkg/dmsg/README.md new file mode 100644 index 0000000000..03da8d685b --- /dev/null +++ b/pkg/dmsg/README.md @@ -0,0 +1,134 @@ +# `dmsg` + +## Terminology + +- **entity -** A service of `dmsg` (typically being part of an executable running on a machine). +- **entity type -** The type of entity. `dmsg` has three entity types; `dmsg.Server`, `dmsg.Client`, `dmsg.Discovery`. +- **entry -** A data structure that describes an entity and is stored in `dmsg.Discovery` for entities to access. +- **frame -** The data unit of the `dmsg` system. +- **frame type -** The type of `dmsg` frame. There are four frame types; `REQUEST`, `ACCEPT`, `CLOSE`, `FWD`, `ACK`. +- **connection -** The direct line of duplex communication between a `dmsg.Server` and `dmsg.Client`. +- **transport -** A line of communication between two `dmsg.Client`s that is proxied via a `dmsg.Server`. +- **transport ID -** A uint16 value that identifies a transport. + +## Frames + +Frames have two sections; the header and the payload. + +``` +|| FrameType | TransportID | PayloadSize || Payload || +|| 1 byte | 2 bytes | 2 bytes || ~ bytes || +``` + + + +## Entities + +The `dmsg` system is made up of three entity types: +- `dmsg.Discovery` is a RESTful API that allows `dmsg.Client`s to find remote `dmg.Client`s and `dmsg.Server`s. +- `dmsg.Server` proxies frames between clients. +- `dmsg.Client` establishes transports between itself and remote `dmsg.Client`s. + +### `dmsg.Discovery` + +Only 3 endpoints need to be defined; Get Entry, Post Entry, and Get Available Servers. + +#### GET Entry + +Obtains a messaging node's entry. + +> `GET {domain}/discovery/entries/{public_key}` + +**REQUEST** + +Header: + +``` +Accept: application/json +``` + +**RESPONSE** + +Possible Status Codes: + +- Success (200) - Successfully updated record. + + - Header: + + ``` + Content-Type: application/json + ``` + + - Body: + + > JSON-encoded entry. + +- Not Found (404) - Entry of public key is not found. + +- Unauthorized (401) - invalid signature. + +- Internal Server Error (500) - something unexpected happened. + +#### POST Entry + +Posts an entry and replaces the current entry if valid. + +> `POST {domain}/discovery/entries` + +**REQUEST** + +Header: + +``` +Content-Type: application/json +``` + +Body: + +> JSON-encoded, signed Entry. + +**RESPONSE** + +Possible Response Codes: + +- Success (200) - Successfully registered record. +- Unauthorized (401) - invalid signature. +- Internal Server Error (500) - something unexpected happened. + +#### GET Available Servers + +Obtains a subset of available server entries. + +> `GET {domain}/discovery/available_servers` + +**REQUEST** + +Header: + +``` +Accept: application/json +``` + +**RESPONSE** + +Possible Status Codes: + +- Success (200) - Got results. + + - Header: + + ``` + Content-Type: application/json + ``` + + - Body: + + > JSON-encoded `[]Entry`. + +- Not Found (404) - No results. + +- Forbidden (403) - When access is forbidden. + +- Internal Server Error (500) - Something unexpected happened. + + diff --git a/pkg/dmsg/testing.md b/pkg/dmsg/TESTING.md similarity index 100% rename from pkg/dmsg/testing.md rename to pkg/dmsg/TESTING.md diff --git a/pkg/dmsg/frame.go b/pkg/dmsg/frame.go index 1c68fab684..a4f630fd8a 100644 --- a/pkg/dmsg/frame.go +++ b/pkg/dmsg/frame.go @@ -61,11 +61,11 @@ func (ft FrameType) String() string { // Frame types. const ( - RequestType = FrameType(1) - AcceptType = FrameType(2) - CloseType = FrameType(3) - FwdType = FrameType(10) - AckType = FrameType(11) + RequestType = FrameType(0x1) + AcceptType = FrameType(0x2) + CloseType = FrameType(0x3) + FwdType = FrameType(0xa) + AckType = FrameType(0xb) ) // Frame is the dmsg data unit. From 149844b79a08c9c3a65109d079064d957c244fe7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E5=BF=97=E5=AE=87?= Date: Sun, 16 Jun 2019 00:09:59 +0800 Subject: [PATCH 4/4] Fixed Client.Close logic. --- pkg/dmsg/client.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/pkg/dmsg/client.go b/pkg/dmsg/client.go index b70a967415..660ce15b28 100644 --- a/pkg/dmsg/client.go +++ b/pkg/dmsg/client.go @@ -159,15 +159,7 @@ func (c *ClientConn) Serve(ctx context.Context, accept chan<- *Transport) (err e log := c.log.WithField("remoteServer", c.remoteSrv) log.WithField("connCount", incrementServeCount()).Infoln("ServingConn") defer func() { - log.WithError(err).WithField("connCount", decrementServeCount()).Infoln("ClosingConn") - c.mx.Lock() - for _, tp := range c.tps { - if tp != nil { - go tp.Close() //nolint:errcheck - } - } - _ = c.Conn.Close() //nolint:errcheck - c.mx.Unlock() + log.WithError(err).WithField("connCount", decrementServeCount()).Infoln("ConnectionClosed") c.wg.Done() }() @@ -255,8 +247,17 @@ func (c *ClientConn) Close() error { closed = true c.log.WithField("remoteServer", c.remoteSrv).Infoln("ClosingConnection") close(c.done) + c.mx.Lock() + for _, tp := range c.tps { + if tp != nil { + go tp.Close() //nolint:errcheck + } + } + _ = c.Conn.Close() //nolint:errcheck + c.mx.Unlock() + c.wg.Wait() }) - c.wg.Wait() + if !closed { return ErrClientClosed }