[Feature] Batch extension should not work with a copy (#313)

Author: informalict

MAINTAINERS.md

@@ -8,4 +8,6 @@
   - `make run-tests-single`
   - `make run-tests-resilientsingle`
   - `make run-tests-cluster`.
+- The test can be launched with the flag `RACE=on` which means that test will be performed with the race detector, e.g:
+  - `RACE=on make run-tests-single`
 - Always create changes in a PR

Makefile

@@ -24,6 +24,13 @@ ifdef VERBOSE
 	TESTVERBOSEOPTIONS := -v
 endif
 
+CGO_ENABLED=0
+ifdef RACE
+	TESTVERBOSEOPTIONS += -race
+	CGO_ENABLED=1
+endif
+
+
 ORGPATH := github.com/arangodb
 REPONAME := $(PROJECT)
 REPODIR := $(ORGDIR)/$(REPONAME)
@@ -142,7 +149,7 @@ run-unit-tests:
 	@$(DOCKER_CMD) \
 		--rm \
 		-v "${ROOTDIR}":/usr/code \
-		-e CGO_ENABLED=0 \
+		-e CGO_ENABLED=$(CGO_ENABLED) \
 		-w /usr/code/ \
 		$(GOIMAGE) \
 		go test $(TESTOPTIONS) $(REPOPATH)/http $(REPOPATH)/agency
@@ -352,7 +359,7 @@ __test_go_test:
 		-e TEST_BACKUP_REMOTE_CONFIG='$(TEST_BACKUP_REMOTE_CONFIG)' \
 		-e TEST_DEBUG='$(TEST_DEBUG)' \
 		-e GODEBUG=tls13=1 \
-		-e CGO_ENABLED=0 \
+		-e CGO_ENABLED=$(CGO_ENABLED) \
 		-w /usr/code/ \
 		$(GOIMAGE) \
 		go test $(GOBUILDTAGSOPT) $(TESTOPTIONS) $(TESTVERBOSEOPTIONS) $(TESTS)
@@ -373,7 +380,7 @@ __test_v2_go_test:
 		-e TEST_BACKUP_REMOTE_CONFIG='$(TEST_BACKUP_REMOTE_CONFIG)' \
 		-e TEST_DEBUG='$(TEST_DEBUG)' \
 		-e GODEBUG=tls13=1 \
-		-e CGO_ENABLED=0 \
+		-e CGO_ENABLED=$(CGO_ENABLED) \
 		-w /usr/code/v2/ \
 		$(GOV2IMAGE) \
 		go test $(GOBUILDTAGSOPT) $(TESTOPTIONS) $(TESTVERBOSEOPTIONS) ./tests

replication_impl.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2018 ArangoDB GmbH, Cologne, Germany
+// Copyright 2018-2021 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
 // Copyright holder is ArangoDB GmbH, Cologne, Germany
 //
 // Author Ewout Prangsma
+// Author Tomasz Mielech
 //
 
 package driver
@@ -110,7 +111,7 @@ func (b batchMetadata) LastTick() Tick {
 }
 
 // Extend the lifetime of an existing batch on the server
-func (b batchMetadata) Extend(ctx context.Context, ttl time.Duration) error {
+func (b *batchMetadata) Extend(ctx context.Context, ttl time.Duration) error {
 	if !atomic.CompareAndSwapInt32(&b.closed, 0, 0) {
 		return WithStack(ErrBatchClosed)
 	}

test/replication_test.go

@@ -18,15 +18,20 @@
 // Copyright holder is ArangoDB GmbH, Cologne, Germany
 //
 // Author Ewout Prangsma
+// Author Tomasz Mielech
 //
 
 package test
 
 import (
 	"context"
+	"errors"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	driver "github.com/arangodb/go-driver"
 )
 
@@ -85,3 +90,53 @@ func TestReplicationDatabaseInventory(t *testing.T) {
 		}
 	}
 }
+
+func TestReplicationBatch(t *testing.T) {
+	ctx := context.Background()
+	c := createClientFromEnv(t, true)
+
+	if _, err := c.Cluster(ctx); err == nil {
+		// Cluster, not supported for this test
+		t.Skip("Skipping in cluster")
+	} else if !driver.IsPreconditionFailed(err) {
+		t.Errorf("Failed to query cluster: %s", describe(err))
+	}
+
+	rep := c.Replication()
+	db, err := c.Database(ctx, "_system")
+	require.NoError(t, err, "failed to open _system database")
+
+	var serverID int64 = 1338 // Random test value
+	batch, err := rep.CreateBatch(ctx, db, serverID, time.Second*60)
+	require.NoError(t, err, "can not create a batch")
+
+	ctxCancel, cancel := context.WithCancel(ctx)
+	errExtend := make(chan error)
+	go func(channel chan<- error) {
+		for {
+			select {
+			case <-ctxCancel.Done():
+				// The batch should be closed.
+				channel <- batch.Extend(ctx, time.Second*60)
+				return
+			default:
+				// Extend the batch immediately.
+				if e := batch.Extend(ctx, time.Second*60); e == driver.ErrBatchClosed {
+					if ctxError := ctx.Err(); ctxError != nil && ctxError != context.Canceled {
+						channel <- errors.New("the batch extension is closed before it is expected")
+						return
+					}
+				}
+			}
+		}
+	}(errExtend)
+
+	// Extend the created batch for 1 second.
+	time.Sleep(time.Second * 1)
+
+	// Delete the batch and interrupt the Go routine for batch extension.
+	err = batch.Delete(ctx)
+	cancel()
+	assert.NoError(t, err, "can not delete a batch")
+	require.EqualError(t, <-errExtend, driver.ErrBatchClosed.Error(), "batch should be already closed")
+}