Skip to content

Commit 1cbacd1

Browse files
committed
Prevent syntetic CREATED events on HostSelectionPolicy.KeyspaceChanged during session init
This patch fixes regression introduced in CASSGO-104 that caused the driver to fire syntetic CREATED events on HostSelectionPolicy.KeyspaceChanged for each existing keyspace during session initialization.
1 parent 1920205 commit 1cbacd1

8 files changed

Lines changed: 160 additions & 29 deletions

File tree

cluster.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,8 @@ func (cfg *ClusterConfig) filterHost(host *HostInfo) bool {
409409
type MetadataConfig struct {
410410
// CacheMode controls how the driver reads and caches schema metadata from Cassandra system tables.
411411
//
412+
// It is required to be enabled for [TokenAwareHostPolicy] and custom host selection policies.
413+
//
412414
// Also, it affects the behavior of schema change listeners.
413415
//
414416
// If CacheMode is [KeyspaceOnly], only [KeyspaceChangeListener] will be notified,

common_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,10 @@ func randomText(size int) string {
303303
return string(result)
304304
}
305305

306+
func randomNameWithPrefix(prefix string) string {
307+
return prefix + strings.ToLower(randomText(10))
308+
}
309+
306310
func assertEqual(t *testing.T, description string, expected, actual interface{}) {
307311
t.Helper()
308312
if expected != actual {

integration_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,11 @@ import (
3535
"net"
3636
"reflect"
3737
"strings"
38+
"sync"
3839
"testing"
3940
"time"
4041

42+
"github.com/stretchr/testify/require"
4143
inf "gopkg.in/inf.v0"
4244
)
4345

@@ -1028,3 +1030,87 @@ func TestSmallTimeoutNoPoolErrors(t *testing.T) {
10281030
errorCount, logOutput)
10291031
}
10301032
}
1033+
1034+
// Emulates round-robin host selection policy and captures KeyspaceChanged events.
1035+
type capturingRRTestPolicy struct {
1036+
roundRobinHostPolicy
1037+
1038+
capturedEvents []KeyspaceUpdateEvent
1039+
mu sync.Mutex
1040+
}
1041+
1042+
func (c *capturingRRTestPolicy) KeyspaceChanged(event KeyspaceUpdateEvent) {
1043+
c.mu.Lock()
1044+
defer c.mu.Unlock()
1045+
c.capturedEvents = append(c.capturedEvents, event)
1046+
c.roundRobinHostPolicy.KeyspaceChanged(event)
1047+
}
1048+
1049+
func (c *capturingRRTestPolicy) CapturedEvents() []KeyspaceUpdateEvent {
1050+
c.mu.Lock()
1051+
defer c.mu.Unlock()
1052+
return append([]KeyspaceUpdateEvent{}, c.capturedEvents...)
1053+
}
1054+
1055+
func (c *capturingRRTestPolicy) ResetEvents() {
1056+
c.mu.Lock()
1057+
defer c.mu.Unlock()
1058+
c.capturedEvents = nil
1059+
}
1060+
1061+
func TestHostSelectionPolicyKeyspaceChangedEvents(t *testing.T) {
1062+
cluster := createCluster()
1063+
policy := capturingRRTestPolicy{}
1064+
cluster.Metadata.CacheMode = Full
1065+
cluster.PoolConfig.HostSelectionPolicy = &policy
1066+
1067+
session, err := cluster.CreateSession()
1068+
require.NoError(t, err)
1069+
defer session.Close()
1070+
1071+
// Regression test for CASSGO-132: KeyspaceChanged events are fired during session creation.
1072+
events := policy.CapturedEvents()
1073+
require.Empty(t, events, "KeyspaceChanged events should not be fired during session creation")
1074+
1075+
// Check keyspace events are fired
1076+
1077+
// CREATE
1078+
ksName := randomNameWithPrefix("gocql_test_")
1079+
err = session.Query(fmt.Sprintf("CREATE KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'}", ksName)).ExecContext(context.Background())
1080+
require.NoError(t, err)
1081+
1082+
require.Eventually(t, func() bool {
1083+
events = policy.CapturedEvents()
1084+
return len(events) == 1
1085+
}, time.Second*5, time.Millisecond*100)
1086+
1087+
require.Len(t, events, 1)
1088+
require.Equal(t, ksName, events[0].Keyspace)
1089+
require.Equal(t, SchemaChangeTypeCreated, events[0].Change)
1090+
1091+
// ALTER
1092+
policy.ResetEvents()
1093+
err = session.Query(fmt.Sprintf("ALTER KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '2'}", ksName)).ExecContext(context.Background())
1094+
require.NoError(t, err)
1095+
require.Eventually(t, func() bool {
1096+
events = policy.CapturedEvents()
1097+
return len(events) == 1
1098+
}, time.Second*5, time.Millisecond*100)
1099+
1100+
require.Len(t, events, 1)
1101+
require.Equal(t, ksName, events[0].Keyspace)
1102+
require.Equal(t, SchemaChangeTypeUpdated, events[0].Change)
1103+
1104+
// DROP
1105+
policy.ResetEvents()
1106+
err = session.Query(fmt.Sprintf("DROP KEYSPACE %s", ksName)).ExecContext(context.Background())
1107+
require.NoError(t, err)
1108+
require.Eventually(t, func() bool {
1109+
events = policy.CapturedEvents()
1110+
return len(events) == 1
1111+
}, time.Second*5, time.Millisecond*100)
1112+
1113+
require.Len(t, events, 1)
1114+
require.Equal(t, ksName, events[0].Keyspace)
1115+
require.Equal(t, SchemaChangeTypeDropped, events[0].Change)
1116+
}

metadata.go

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,10 @@ type schemaDescriber struct {
575575
// Schema change events are server-initiated messages sent to clients that have registered
576576
// for schema change notifications. These events indicate modifications to keyspaces, tables,
577577
// user-defined types, functions, or aggregates.
578+
//
579+
// When a schema change event is received, the driver will update its internal metadata
580+
// by comparing the old and new metadata. If the metadata has changed, the driver will
581+
// notify [HostSelectionPolicy.KeyspaceChanged] method about the change.
578582
const (
579583
SchemaChangeTypeCreated = "CREATED" // Schema object was created
580584
SchemaChangeTypeUpdated = "UPDATED" // Schema object was modified
@@ -858,9 +862,6 @@ func refreshSchemas(session *Session) error {
858862

859863
// Notify policy if it supports schema refresh notifications
860864
notifier, supportsRefresh := session.policy.(schemaRefreshNotifier)
861-
hasKeyspaceListener := session.schemaListeners.hasKeyspace() && sessionInitialized
862-
863-
// Notify the policy if it supports schema refresh notifications
864865
if supportsRefresh {
865866
notifier.schemaRefreshed(session.schemaDescriber.getSchemaMetaForRead())
866867
}
@@ -871,31 +872,36 @@ func refreshSchemas(session *Session) error {
871872
NewLogFieldInt("dropped_keyspaces_count", len(droppedKeyspaces)),
872873
)
873874

874-
// If we don't support schemaRefreshed OR we have listeners, we need to loop
875-
if !supportsRefresh || hasKeyspaceListener {
875+
// We should not call KeyspaceChanged method if policy implements schemaRefreshNotifier.
876+
shouldCallKeyspaceChanged := !supportsRefresh && sessionInitialized
877+
// We should not notify keyspace listeners if they are not set or session is not initialized yet.
878+
shouldNotifyKeyspaceListener := session.schemaListeners.hasKeyspace() && sessionInitialized
879+
880+
// Loop only if we need to notify policy or keyspace listeners.
881+
if shouldCallKeyspaceChanged || shouldNotifyKeyspaceListener {
876882
for _, name := range newKeyspaces {
877-
if !supportsRefresh {
883+
if shouldCallKeyspaceChanged {
878884
session.policy.KeyspaceChanged(KeyspaceUpdateEvent{Keyspace: name, Change: SchemaChangeTypeCreated})
879885
}
880-
if hasKeyspaceListener {
886+
if shouldNotifyKeyspaceListener {
881887
session.schemaListeners.OnKeyspaceCreated(OnKeyspaceCreatedEvent{Keyspace: keyspaces[name].Clone()})
882888
}
883889
}
884890

885891
for _, name := range droppedKeyspaces {
886-
if !supportsRefresh {
892+
if shouldCallKeyspaceChanged {
887893
session.policy.KeyspaceChanged(KeyspaceUpdateEvent{Keyspace: name, Change: SchemaChangeTypeDropped})
888894
}
889-
if hasKeyspaceListener {
895+
if shouldNotifyKeyspaceListener {
890896
session.schemaListeners.OnKeyspaceDropped(OnKeyspaceDroppedEvent{Keyspace: oldKeyspaceMeta[name].Clone()})
891897
}
892898
}
893899

894900
for _, name := range updatedKeyspaces {
895-
if !supportsRefresh {
901+
if shouldCallKeyspaceChanged {
896902
session.policy.KeyspaceChanged(KeyspaceUpdateEvent{Keyspace: name, Change: SchemaChangeTypeUpdated})
897903
}
898-
if hasKeyspaceListener {
904+
if shouldNotifyKeyspaceListener {
899905
session.schemaListeners.OnKeyspaceUpdated(OnKeyspaceUpdatedEvent{
900906
Old: oldKeyspaceMeta[name].Clone(),
901907
New: keyspaces[name].Clone(),

policies.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,9 @@ func NonLocalReplicasFallback() func(policy *tokenAwareHostPolicy) {
427427
}
428428

429429
// ShuffledTokenAwareHostPolicy is a token aware host selection policy that shuffles replicas.
430+
//
431+
// Note: TokenAwareHostPolicy requires the metadata cache to be enabled.
432+
// See [ClusterConfig.Metadata] for more details.
430433
func ShuffledTokenAwareHostPolicy(fallback HostSelectionPolicy, opts ...func(*tokenAwareHostPolicy)) HostSelectionPolicy {
431434
p := &tokenAwareHostPolicy{
432435
fallback: fallback,
@@ -442,6 +445,9 @@ func ShuffledTokenAwareHostPolicy(fallback HostSelectionPolicy, opts ...func(*to
442445
// TokenAwareHostPolicy is a token aware host selection policy, where hosts are
443446
// selected based on the partition key, so queries are sent to the host which
444447
// owns the partition. Fallback is used when routing information is not available.
448+
//
449+
// Note: TokenAwareHostPolicy requires the metadata cache to be enabled.
450+
// See [ClusterConfig.Metadata] for more details.
445451
func TokenAwareHostPolicy(fallback HostSelectionPolicy, opts ...func(*tokenAwareHostPolicy)) HostSelectionPolicy {
446452
p := &tokenAwareHostPolicy{fallback: fallback}
447453
for _, opt := range opts {
@@ -1080,3 +1086,16 @@ type SimpleSpeculativeExecution struct {
10801086

10811087
func (sp *SimpleSpeculativeExecution) Attempts() int { return sp.NumAttempts }
10821088
func (sp *SimpleSpeculativeExecution) Delay() time.Duration { return sp.TimeoutDelay }
1089+
1090+
// hostSelectionPolicyRequiresMetadata returns true if the host selection policy requires the metadata cache to be enabled.
1091+
// [tokenAwareHostPolicy] is the only builtin policy that requires the metadata cache to be enabled.
1092+
// Custom policies also require the metadata cache to be enabled.
1093+
func hostSelectionPolicyRequiresMetadata(p HostSelectionPolicy) bool {
1094+
switch p.(type) {
1095+
case *rackAwareRR, *roundRobinHostPolicy, *singleHostReadyPolicy, *dcAwareRR:
1096+
return false
1097+
default:
1098+
// tokenAwareHostPolicy and custom policies are required to have the metadata cache enabled.
1099+
return true
1100+
}
1101+
}

policies_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ import (
3737
"strings"
3838
"testing"
3939
"time"
40+
41+
"github.com/stretchr/testify/assert"
4042
)
4143

4244
// Tests of the round-robin host selection policy implementation
@@ -1099,3 +1101,14 @@ func TestHostPolicy_TokenAware_TopologyChangeUpdatesAllKeyspaces(t *testing.T) {
10991101
}
11001102
})
11011103
}
1104+
1105+
func Test_hostSelectionPolicyRequiresMetadataCache(t *testing.T) {
1106+
// Doesn't require metadata cache
1107+
assert.False(t, hostSelectionPolicyRequiresMetadata(RoundRobinHostPolicy()))
1108+
assert.False(t, hostSelectionPolicyRequiresMetadata(DCAwareRoundRobinPolicy("dc1")))
1109+
assert.False(t, hostSelectionPolicyRequiresMetadata(RackAwareRoundRobinPolicy("dc1", "rack1")))
1110+
assert.False(t, hostSelectionPolicyRequiresMetadata(SingleHostReadyPolicy(RoundRobinHostPolicy())))
1111+
1112+
// Requires metadata cache
1113+
assert.True(t, hostSelectionPolicyRequiresMetadata(TokenAwareHostPolicy(RoundRobinHostPolicy())))
1114+
}

schema_events_test.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ package gocql
2323

2424
import (
2525
"fmt"
26-
"strings"
2726
"testing"
2827
"time"
2928

@@ -263,10 +262,6 @@ func testSchemaEventsKeyspace(t *testing.T, session *Session, listener *schemaCh
263262
require.Equal(t, ks, listener.KeyspaceDroppedEvents[0].Keyspace.Name, "Expected keyspace dropped event to have correct keyspace name")
264263
}
265264

266-
func randomNameWithPrefix(prefix string) string {
267-
return prefix + strings.ToLower(randomText(10))
268-
}
269-
270265
func testSchemaEventsTable(t *testing.T, session *Session, listener *schemaChangesTestListener) {
271266
table := randomNameWithPrefix("gocql_table_events_")
272267

session.go

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,14 @@ func NewSession(cfg ClusterConfig) (*Session, error) {
149149
return nil, fmt.Errorf("the default SerialConsistency level is not allowed to be anything else but SERIAL or LOCAL_SERIAL. Recived value: %v", cfg.SerialConsistency)
150150
}
151151

152+
if cfg.PoolConfig.HostSelectionPolicy == nil {
153+
cfg.PoolConfig.HostSelectionPolicy = RoundRobinHostPolicy()
154+
}
155+
156+
if cfg.Metadata.CacheMode == Disabled && hostSelectionPolicyRequiresMetadata(cfg.PoolConfig.HostSelectionPolicy) {
157+
return nil, ErrMetadataCacheRequired
158+
}
159+
152160
// TODO: we should take a context in here at some point
153161
ctx, cancel := context.WithCancel(context.TODO())
154162

@@ -222,9 +230,6 @@ func NewSession(cfg ClusterConfig) (*Session, error) {
222230
}
223231
s.connCfg = connCfg
224232

225-
if cfg.PoolConfig.HostSelectionPolicy == nil {
226-
cfg.PoolConfig.HostSelectionPolicy = RoundRobinHostPolicy()
227-
}
228233
s.pool = cfg.PoolConfig.buildPool(s)
229234
s.policy = cfg.PoolConfig.HostSelectionPolicy
230235

@@ -2517,16 +2522,17 @@ func (e Error) Error() string {
25172522
}
25182523

25192524
var (
2520-
ErrNotFound = errors.New("not found")
2521-
ErrUnavailable = errors.New("unavailable")
2522-
ErrUnsupported = errors.New("feature not supported")
2523-
ErrTooManyStmts = errors.New("too many statements")
2524-
ErrUseStmt = errors.New("use statements aren't supported. Please see https://github.com/apache/cassandra-gocql-driver for explanation.")
2525-
ErrSessionClosed = errors.New("session has been closed")
2526-
ErrNoConnections = errors.New("gocql: no hosts available in the pool")
2527-
ErrNoKeyspace = errors.New("no keyspace provided")
2528-
ErrKeyspaceDoesNotExist = errors.New("keyspace does not exist")
2529-
ErrNoMetadata = errors.New("no metadata available")
2525+
ErrNotFound = errors.New("not found")
2526+
ErrUnavailable = errors.New("unavailable")
2527+
ErrUnsupported = errors.New("feature not supported")
2528+
ErrTooManyStmts = errors.New("too many statements")
2529+
ErrUseStmt = errors.New("use statements aren't supported. Please see https://github.com/apache/cassandra-gocql-driver for explanation.")
2530+
ErrSessionClosed = errors.New("session has been closed")
2531+
ErrNoConnections = errors.New("gocql: no hosts available in the pool")
2532+
ErrNoKeyspace = errors.New("no keyspace provided")
2533+
ErrKeyspaceDoesNotExist = errors.New("keyspace does not exist")
2534+
ErrNoMetadata = errors.New("no metadata available")
2535+
ErrMetadataCacheRequired = errors.New("metadata cache must be enabled for the selected host selection policy")
25302536
)
25312537

25322538
// ErrProtocol represents a protocol-level error.

0 commit comments

Comments
 (0)