Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CBG-3700 fix race conditions #6633

Merged
merged 11 commits into from
Jan 10, 2024
248 changes: 125 additions & 123 deletions base/stats.go

Large diffs are not rendered by default.

7 changes: 6 additions & 1 deletion db/import_pindex.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,20 @@ import (
"context"
"errors"
"fmt"
"sync"

"github.com/couchbase/cbgt"
"github.com/couchbase/sync_gateway/base"
)

// registerImportPindexImplMutex locks access to cbgt.RegisterImportPindexImpl.
var registerImportPindexImplMutex = sync.Mutex{}

// RegisterImportPindexImpl registers the PIndex type definition. This is invoked by cbgt when a Pindex (collection of
// vbuckets) is assigned to this node.

func RegisterImportPindexImpl(ctx context.Context, configGroup string) {
registerImportPindexImplMutex.Lock()
defer registerImportPindexImplMutex.Unlock()

// Since RegisterPIndexImplType is a global var without synchronization, index type needs to be
// config group scoped. The associated importListener within the context is retrieved based on the
Expand Down
2 changes: 1 addition & 1 deletion rest/adminapitest/admin_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3108,9 +3108,9 @@ func TestNotExistentDBRequest(t *testing.T) {

func TestConfigsIncludeDefaults(t *testing.T) {
base.RequireNumTestBuckets(t, 2)

base.SetUpTestLogging(t, base.LevelInfo, base.KeyHTTP)

defer rest.RunBootstrapLoggerInitialization(t)()
ctx := base.TestCtx(t)
// Get a test bucket, to use to create the database.
tb := base.GetTestBucket(t)
Expand Down
8 changes: 2 additions & 6 deletions rest/adminapitest/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,11 @@ licenses/APL2.txt.
package adminapitest

import (
"context"
"testing"

"github.com/couchbase/sync_gateway/base"
"github.com/couchbase/sync_gateway/db"
"github.com/couchbase/sync_gateway/rest"
)

func TestMain(m *testing.M) {
ctx := context.Background() // start of test process
tbpOptions := base.TestBucketPoolOptions{MemWatermarkThresholdMB: 8192}
db.TestBucketPoolWithIndexes(ctx, m, tbpOptions)
rest.TestBucketPool(m)
}
8 changes: 2 additions & 6 deletions rest/attachmentcompactiontest/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,11 @@ licenses/APL2.txt.
package attachmentcompactiontest

import (
"context"
"testing"

"github.com/couchbase/sync_gateway/base"
"github.com/couchbase/sync_gateway/db"
"github.com/couchbase/sync_gateway/rest"
)

func TestMain(m *testing.M) {
ctx := context.Background() // start of test process
tbpOptions := base.TestBucketPoolOptions{MemWatermarkThresholdMB: 2048}
db.TestBucketPoolWithIndexes(ctx, m, tbpOptions)
rest.TestBucketPool(m)
}
8 changes: 2 additions & 6 deletions rest/changestest/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,11 @@ licenses/APL2.txt.
package changestest

import (
"context"
"testing"

"github.com/couchbase/sync_gateway/base"
"github.com/couchbase/sync_gateway/db"
"github.com/couchbase/sync_gateway/rest"
)

func TestMain(m *testing.M) {
ctx := context.Background() // start of test process
tbpOptions := base.TestBucketPoolOptions{MemWatermarkThresholdMB: 2048}
db.TestBucketPoolWithIndexes(ctx, m, tbpOptions)
rest.TestBucketPool(m)
}
24 changes: 14 additions & 10 deletions rest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ var (

// The value of defaultLogFilePath is populated by -defaultLogFilePath by command line flag from service scripts.
defaultLogFilePath string

// loggerInitialized is used to track whether the logger has been initialized, useful to avoid reinitialization in the test harness.
loggerInitialized = false
)

const (
Expand Down Expand Up @@ -1204,7 +1207,6 @@ func envDefaultExpansion(ctx context.Context, key string, getEnvFn func(string)

// SetupAndValidateLogging validates logging config and initializes all logging.
func (sc *StartupConfig) SetupAndValidateLogging(ctx context.Context) (err error) {

base.SetRedaction(sc.Logging.RedactionLevel)

if sc.Logging.LogFilePath == "" {
Expand Down Expand Up @@ -1344,16 +1346,18 @@ func (sc *StartupConfig) Validate(ctx context.Context, isEnterpriseEdition bool)

// SetupServerContext creates a new ServerContext given its configuration and performs the context validation.
func SetupServerContext(ctx context.Context, config *StartupConfig, persistentConfig bool) (*ServerContext, error) {
// Logging config will now have been loaded from command line
// or from a sync_gateway config file so we can validate the
// configuration and setup logging now
if err := config.SetupAndValidateLogging(ctx); err != nil {
// If we didn't set up logging correctly, we *probably* can't log via normal means...
// as a best-effort, last-ditch attempt, we'll log to stderr as well.
log.Printf("[ERR] Error setting up logging: %v", err)
return nil, fmt.Errorf("error setting up logging: %v", err)
// logger will be initialized only when running tests
if !loggerInitialized {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't feel like quite the right place to be using this variable - wouldn't we'd want to encapsulate it in the logging handling (inside SetupAndValidateLogging)?

We'd also still need some sort of synchronization to avoid races.

// Logging config will now have been loaded from command line
// or from a sync_gateway config file so we can validate the
// configuration and setup logging now
if err := config.SetupAndValidateLogging(ctx); err != nil {
// If we didn't set up logging correctly, we *probably* can't log via normal means...
// as a best-effort, last-ditch attempt, we'll log to stderr as well.
log.Printf("[ERR] Error setting up logging: %v", err)
return nil, fmt.Errorf("error setting up logging: %v", err)
}
}

base.FlushLoggerBuffers()

base.InfofCtx(ctx, base.KeyAll, "Logging: Console level: %v", base.ConsoleLogLevel())
Expand Down
8 changes: 2 additions & 6 deletions rest/functionsapitest/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,11 @@ licenses/APL2.txt.
package functionsapitest

import (
"context"
"testing"

"github.com/couchbase/sync_gateway/base"
"github.com/couchbase/sync_gateway/db"
"github.com/couchbase/sync_gateway/rest"
)

func TestMain(m *testing.M) {
ctx := context.Background() // start of test process
tbpOptions := base.TestBucketPoolOptions{MemWatermarkThresholdMB: 18192}
db.TestBucketPoolWithIndexes(ctx, m, tbpOptions)
rest.TestBucketPool(m)
}
7 changes: 2 additions & 5 deletions rest/importtest/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,15 @@ licenses/APL2.txt.
package importtest

import (
"context"
"testing"

"github.com/couchbase/sync_gateway/base"
"github.com/couchbase/sync_gateway/db"
"github.com/couchbase/sync_gateway/rest"
)

func TestMain(m *testing.M) {
if !base.TestUseXattrs() { // import tests only run if xattrs are enabled
return
}
ctx := context.Background() // start of test process
tbpOptions := base.TestBucketPoolOptions{MemWatermarkThresholdMB: 2048}
db.TestBucketPoolWithIndexes(ctx, m, tbpOptions)
rest.TestBucketPool(m)
}
8 changes: 2 additions & 6 deletions rest/importuserxattrtest/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,16 @@ licenses/APL2.txt.
package importuserxattrtest

import (
"context"
"testing"

"github.com/couchbase/sync_gateway/base"
"github.com/couchbase/sync_gateway/db"
"github.com/couchbase/sync_gateway/rest"
)

func TestMain(m *testing.M) {
// user xattr tests require xattrs and EE
if !base.TestUseXattrs() || !base.IsEnterpriseEdition() {
return
}

ctx := context.Background() // start of test process
tbpOptions := base.TestBucketPoolOptions{MemWatermarkThresholdMB: 2048}
db.TestBucketPoolWithIndexes(ctx, m, tbpOptions)
rest.TestBucketPool(m)
}
8 changes: 2 additions & 6 deletions rest/indextest/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,11 @@ licenses/APL2.txt.
package indextest

import (
"context"
"testing"

"github.com/couchbase/sync_gateway/base"
"github.com/couchbase/sync_gateway/rest"
)

func TestMain(m *testing.M) {
ctx := context.Background() // start of test process
tbpOptions := base.TestBucketPoolOptions{MemWatermarkThresholdMB: 2048}
// Do not create indexes for this test, so they are built by server_context.go
base.TestBucketPoolNoIndexes(ctx, m, tbpOptions)
rest.TestBucketPool(m)
}
6 changes: 1 addition & 5 deletions rest/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,15 @@ licenses/APL2.txt.
package rest

import (
"context"
"testing"

"github.com/couchbase/sync_gateway/base"
"github.com/couchbase/sync_gateway/db"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestMain(m *testing.M) {
ctx := context.Background() // start of test process
tbpOptions := base.TestBucketPoolOptions{MemWatermarkThresholdMB: 8192}
db.TestBucketPoolWithIndexes(ctx, m, tbpOptions)
TestBucketPool(m)
}

func TestConfigOverwritesLegacyFlags(t *testing.T) {
Expand Down
8 changes: 2 additions & 6 deletions rest/replicatortest/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,11 @@ licenses/APL2.txt.
package replicatortest

import (
"context"
"testing"

"github.com/couchbase/sync_gateway/base"
"github.com/couchbase/sync_gateway/db"
"github.com/couchbase/sync_gateway/rest"
)

func TestMain(m *testing.M) {
ctx := context.Background() // start of test process
tbpOptions := base.TestBucketPoolOptions{MemWatermarkThresholdMB: 8192}
db.TestBucketPoolWithIndexes(ctx, m, tbpOptions)
rest.TestBucketPool(m)
}
8 changes: 8 additions & 0 deletions rest/utilities_testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -2649,3 +2649,11 @@ func RequireGocbDCPResync(t *testing.T) {
t.Skip("This test only works against Couchbase Server since rosmar has no support for DCP resync")
}
}

// TestBucketPool creates a bucket pool used for rest and its subpackages
func TestBucketPool(m *testing.M) {
ctx := context.Background() // start of test process
loggerInitialized = true
tbpOptions := base.TestBucketPoolOptions{MemWatermarkThresholdMB: 8192}
db.TestBucketPoolWithIndexes(ctx, m, tbpOptions)
}
6 changes: 6 additions & 0 deletions rest/utilities_testing_bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,9 @@ func StartServerWithConfig(t *testing.T, config *StartupConfig) (*ServerContext,
started = true
return sc, closeFn
}

// RunBootstrapLoggerInitialization forces the logger initialization to run. This code is sensitive to race conditions if another ServerContext is in use.
func RunBootstrapLoggerInitialization(*testing.T) func() {
loggerInitialized = false
return func() { loggerInitialized = true }
}