Merge branch 'master' into retry

Author: meling

.github/workflows/go-test.yml

@@ -9,30 +9,39 @@ jobs:
   test:
     strategy:
       matrix:
-        go-version: [1.16]
+        go-version: ["1.22"]
         platform: [ubuntu-latest, macos-latest]
     runs-on: ${{ matrix.platform }}
+
     steps:
-    - name: Install Go
-      uses: actions/setup-go@v2
-      with:
-        go-version: ${{ matrix.go-version }}
-    - name: Install Protoc
-      uses: arduino/setup-protoc@master
+    - name: Cache Go Modules
+      uses: actions/cache@v3
       with:
-        version: '3.x'
-        repo-token: ${{ secrets.GITHUB_TOKEN }}
-    - uses: actions/cache@v2
-      with:
-        path: ~/go/pkg/mod
+        path: |
+          ~/.cache/go-build          # ubuntu-latest
+          ~/Library/Caches/go-build  # macos-latest
+          ~/go/pkg/mod
         key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
         restore-keys: |
           ${{ runner.os }}-go-
+
     - name: Checkout code
-      uses: actions/checkout@v2
+      uses: actions/checkout@v3
+
+    - name: Set up Go
+      uses: actions/setup-go@v3
+      with:
+        go-version: ${{ matrix.go-version }}
+
+    - name: Install Protoc
+      uses: arduino/setup-protoc@v3.0.0
+      with:
+        repo-token: ${{ secrets.GITHUB_TOKEN }}
+
     - name: Install tools
       run: |
         make tools
         make installgorums
-    - name: Test
+
+    - name: Run Tests
       run: make test

.github/workflows/golangci-lint.yml

@@ -1,49 +1,51 @@
 name: golangci-lint
 on:
   push:
-    branches: [ master ]
+    branches: [master]
   pull_request:
-    branches: [ master ]
+permissions:
+  contents: read
+  # Optional: allow read access to pull request. Use with `only-new-issues` option.
+  pull-requests: read
 
 jobs:
   golangci:
-    strategy:
-      matrix:
-        go-version: [1.16]
+    name: lint
     runs-on: ubuntu-latest
     steps:
-      - name: Install Go
-        uses: actions/setup-go@v2
-        with:
-          go-version: ${{ matrix.go-version }}
-
-      - name: Install Protoc
-        uses: arduino/setup-protoc@master
-        with:
-          version: '3.x'
-          repo-token: ${{ secrets.GITHUB_TOKEN }}
-
-      - name: Cache Go modules
-        uses: actions/cache@v2
+      - name: Cache Go Modules
+        uses: actions/cache@v3
         with:
           path: |
+            ~/.cache/go-build          # ubuntu-latest
+            ~/Library/Caches/go-build  # macos-latest
             ~/go/pkg/mod
-            ~/.cache/go-build
-          key: ${{ runner.os }}-${{ matrix.go-version }}-go-${{ hashFiles('**/go.sum') }}
+          key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
           restore-keys: |
-            ${{ runner.os }}-${{ matrix.go-version }}-go-
+            ${{ runner.os }}-go-
 
-      - name: Checkout 
-        uses: actions/checkout@v2
+      - name: Checkout code
+        uses: actions/checkout@v3
+
+      - name: Set up Go
+        uses: actions/setup-go@v3
+        with:
+          go-version: "1.22"
+
+      - name: Setup Protoc
+        uses: arduino/setup-protoc@v3.0.0
+        with:
+          repo-token: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Run Make
         run: make
 
       - name: Run golangci-lint
-        uses: golangci/golangci-lint-action@v2
+        uses: golangci/golangci-lint-action@v3
         with:
           version: latest
-          skip-go-installation: true
           skip-pkg-cache: true
           skip-build-cache: true
-          args: --disable errcheck
+          args: --timeout 5m --disable errcheck
+          # Optional: show only new issues if it's a pull request. The default value is `false`.
+          only-new-issues: true

.gitignore

@@ -32,10 +32,14 @@ Session.vim
 *~
 
 *.xo
+.vscode/*.log
 
 # binary files
 cmd/benchmark/benchmark
 
 ideas.txt
 
 cmd/protoc-gen-gorums/gengorums/template_static.go.bak
+
+# jetbrains IDE files
+.idea/

.vscode/gorums.txt

@@ -14,6 +14,7 @@ emptypb
 Erevik
 errcheck
 extendee
+Frausing's
 fset
 gengorums
 GOBIN
@@ -28,14 +29,17 @@ gorumsexample
 grpc
 Hein
 ICDCS
+Idents
 Idxs
+Iface
 Ingve
 installgorums
 Jehl
 Lmsgprefix
 Mallocs
 markdownlint
 mcast
+Meland's
 Meling
 memprofile
 multicast
@@ -46,12 +50,15 @@ pkgs
 proto
 protobuf
 protoc
+protogen
 protoimpl
+protoreflect
 ptypes
 QCQF
 qcresult
 qspec
 quorumcall
+Reconf
 relab
 rlevel
 RPC's
@@ -68,6 +75,8 @@ Tormod
 unexport
 Unexported
 unicast
+unmarshal
+unmarshaled
 unmarshaling
 userguide
 Xeon

channel.go

@@ -118,15 +118,26 @@ func (c *channel) sendMsg(req request) (err error) {
 	c.streamMut.RLock()
 	defer c.streamMut.RUnlock()
 
-	done := make(chan struct{}, 1)
+	done := make(chan struct{})
 
-	// wait for either the message to be sent, or the request context being cancelled.
-	// if the request context was cancelled, then we most likely have a blocked stream.
+	// This goroutine waits for either 'done' to be closed, or the request context to be cancelled.
+	// If the request context was cancelled, we have two possibilities:
+	// The stream could be blocked, or the caller could be impatient.
+	// We cannot know which is the case, but it seems wiser to cancel the stream as a precaution,
+	// because reconnection is quite fast and cheap.
 	go func() {
 		select {
 		case <-done:
+			// all is good
 		case <-req.ctx.Done():
-			c.cancelStream()
+			// Both channels could be ready at the same time, so we should check 'done' again.
+			select {
+			case <-done:
+				// false alarm
+			default:
+				// cause reconnect
+				c.cancelStream()
+			}
 		}
 	}()
 
@@ -135,7 +146,8 @@ func (c *channel) sendMsg(req request) (err error) {
 		c.setLastErr(err)
 		c.streamBroken.set()
 	}
-	done <- struct{}{}
+
+	close(done)
 
 	return err
 }

cmd/protoc-gen-gorums/dev/zorums.pb.go

@@ -116,7 +116,7 @@ type servicesData struct {
 	Services []*protogen.Service
 }
 
-// genGorumsType generates Gorums methods and corresponding datastructures for gorumsType.
+// genGorumsType generates Gorums methods and corresponding data structures for gorumsType.
 func genGorumsType(g *protogen.GeneratedFile, services []*protogen.Service, gorumsType string) {
 	data := servicesData{g, services}
 	if callTypeInfo := gorumsCallTypesInfo[gorumsType]; callTypeInfo.extInfo == nil {
@@ -240,7 +240,7 @@ func callTypeName(ext *protoimpl.ExtensionInfo) string {
 // files for the different keys.
 var gorumsCallTypesInfo = map[string]*callTypeInfo{
 	"qspec":  {template: qspecInterface},
-	"types":  {template: datatypes},
+	"types":  {template: dataTypes},
 	"server": {template: server},
 
 	callTypeName(gorums.E_Rpc): {

cmd/protoc-gen-gorums/dev/zorums_async_gorums.pb.go

@@ -56,7 +56,7 @@ func GenerateBundleFile(dst string) {
 		fmt.Printf("\nReview changes above; to revert use:\n")
 		fmt.Printf("mv %s.bak %s\n", dst, dst)
 	}
-	err = os.WriteFile(dst, []byte(staticContent), 0666)
+	err = os.WriteFile(dst, []byte(staticContent), 0o666)
 	if err != nil {
 		log.Fatal(err)
 	}

cmd/protoc-gen-gorums/dev/zorums_correctable_gorums.pb.go

@@ -61,6 +61,6 @@ func (c *{{$correctableOut}}) Get() (*{{$customOut}}, int, error) {
 {{- end -}}
 `
 
-var datatypes = internalOutDataType +
+var dataTypes = internalOutDataType +
 	asyncDataType +
 	correctableDataType

cmd/protoc-gen-gorums/dev/zorums_multicast_gorums.pb.go

@@ -15,11 +15,11 @@ Examples of such layers includes
 
 Frausing's thesis provided two implementations of such layering, one based on interceptors and the metadata support in gRPC, and one where the layer was designed as two separate gRPC services, one with a prefix `g` to separate it from the actual service that applications should use. Please see Frausing's thesis for additional details, and his code on [GitHub](https://github.com/tfrausin/reconf). There is also a question and an answer in this [gRPC issue](https://github.com/grpc/grpc-go/issues/2091).
 
-The metadata approach is not type safe because everything must be converted to a string, and in Frausing's implementation, this requires a custom marshalling and unmarshalling implementation. Moreover, it seems that there is a larger overhead with this approach compared to the alternative with using separate gRPC interfaces.
+The metadata approach is not type safe because everything must be converted to a string, and in Frausing's implementation, this requires a custom marshaling and unmarshaling implementation. Moreover, it seems that there is a larger overhead with this approach compared to the alternative with using separate gRPC interfaces.
 
 ## The interceptor approach
 
-However, it would be interesting to consider (as suggested in the gRPC issue linked above) a generic implementation that will marshal/unmarshal a protobuf structure to/from a json string that can be passed as part of the metadata feature of gRPC. What do I mean by generic here? Well, the content of the metadata to be passed in the metadata structure of an RPC request/response message should be defined as a proto message type, and the marshalling functions should be automatic. Basically, the code generator should produce marshalling functions for the metadata, and a layer-specific method that accepts the request/response object and unmarshalled metadata type as generated by the protobuf for that metadata object.
+However, it would be interesting to consider (as suggested in the gRPC issue linked above) a generic implementation that will marshal/unmarshal a protobuf structure to/from a json string that can be passed as part of the metadata feature of gRPC. What do I mean by generic here? Well, the content of the metadata to be passed in the metadata structure of an RPC request/response message should be defined as a proto message type, and the marshaling functions should be automatic. Basically, the code generator should produce marshaling functions for the metadata, and a layer-specific method that accepts the request/response object and unmarshaled metadata type as generated by the protobuf for that metadata object.
 
 We should essentially generate the interceptor code that extract things and calls out to a function like this:
 
@@ -55,7 +55,7 @@ message Config {
 }
 ```
 
-Note that, we don't need to maintain two separate message types for the actual configuration (list of machines) and the epoch, since if the list of machines is empty, it won't take any space in the message. Empty fields will simply be removed during marshalling.
+Note that, we don't need to maintain two separate message types for the actual configuration (list of machines) and the epoch, since if the list of machines is empty, it won't take any space in the message. Empty fields will simply be removed during marshaling.
 
 On the server-side, our implementation of the reconfiguration layer would look something like this:
 
@@ -145,6 +145,6 @@ Note that the `c.cc` connection points to the reconfiguration layer's `Read` met
 
 1. If our code generator produces these additional layering methods for us, do we need to register the server implementing all interfaces (both `pbm.Config` and `pb.Storage`) or can it be replaced with the top-layer only?
 
-2. Can we support multiple layers? For example, if we want to support both reconfiguration and all-to-all communcation, and these features are best kept separate?
+2. Can we support multiple layers? For example, if we want to support both reconfiguration and all-to-all communication, and these features are best kept separate?
 
 3. The approach above is not so flexible in that a new layer will probably want to define templates for code generation.