Skip to content

Commit 9972e77

Browse files
rodrigozhourodrigozhou
authored
Fix backward compatibility with std visibility when doing search attribute operations (#3907)
1 parent 157cd11 commit 9972e77

File tree

8 files changed

+63
-22
lines changed

8 files changed

+63
-22
lines changed

‎common/persistence/visibility/store/standard/cassandra/visibility_store.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@ type (
140140
visibilityStore struct {
141141
session gocql.Session
142142
lowConslevel gocql.Consistency
143-
keyspace string
144143
}
145144
)
146145

@@ -159,7 +158,6 @@ func NewVisibilityStore(
159158
return &visibilityStore{
160159
session: session,
161160
lowConslevel: gocql.One,
162-
keyspace: cfg.Keyspace,
163161
}, nil
164162
}
165163

@@ -168,7 +166,9 @@ func (v *visibilityStore) GetName() string {
168166
}
169167

170168
func (v *visibilityStore) GetIndexName() string {
171-
return v.keyspace
169+
// GetIndexName is used to get cluster metadata, which in verstions < v1.20
170+
// were stored in an empty string key.
171+
return ""
172172
}
173173

174174
// Close releases the resources held by this object

‎common/persistence/visibility/store/standard/sql/visibility_store.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,9 @@ func (s *visibilityStore) GetName() string {
8181
}
8282

8383
func (s *visibilityStore) GetIndexName() string {
84-
return s.sqlStore.GetDbName()
84+
// GetIndexName is used to get cluster metadata, which in verstions < v1.20
85+
// were stored in an empty string key.
86+
return ""
8587
}
8688

8789
func (s *visibilityStore) RecordWorkflowExecutionStarted(

‎common/persistence/visibility/store/standard/visibility_store.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ func (s *standardStore) GetName() string {
7474
}
7575

7676
func (s *standardStore) GetIndexName() string {
77-
return s.store.GetIndexName()
77+
// GetIndexName is used to get cluster metadata, which in verstions < v1.20
78+
// were stored in an empty string key.
79+
return ""
7880
}
7981

8082
func (s *standardStore) RecordWorkflowExecutionStarted(

‎common/searchattribute/manager.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,11 @@ func (m *managerImpl) GetSearchAttributes(
125125
if indexName != "" {
126126
indexSearchAttributes, ok = saCache.searchAttributes[""]
127127
if ok {
128-
maps.Copy(result.customSearchAttributes, indexSearchAttributes.customSearchAttributes)
128+
if result.customSearchAttributes == nil {
129+
result.customSearchAttributes = maps.Clone(indexSearchAttributes.customSearchAttributes)
130+
} else {
131+
maps.Copy(result.customSearchAttributes, indexSearchAttributes.customSearchAttributes)
132+
}
129133
}
130134
}
131135
return result, nil

‎service/frontend/adminHandler.go

+18-3
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,12 @@ func (adh *AdminHandler) AddSearchAttributes(
270270
}
271271
}
272272

273-
if adh.visibilityMgr.GetName() == elasticsearch.PersistenceName {
273+
// TODO (rodrigozhou): Remove condition `indexName == ""`.
274+
// If indexName == "", then calling addSearchAttributesElasticsearch will
275+
// register the search attributes in the cluster metadata if ES is up or if
276+
// `skip-schema-update` is set. This is for backward compatibility using
277+
// standard visibility.
278+
if adh.visibilityMgr.GetName() == elasticsearch.PersistenceName || indexName == "" {
274279
err = adh.addSearchAttributesElasticsearch(ctx, request, indexName)
275280
} else {
276281
err = adh.addSearchAttributesSQL(ctx, request, currentSearchAttributes)
@@ -404,7 +409,12 @@ func (adh *AdminHandler) RemoveSearchAttributes(
404409
}
405410

406411
var err error
407-
if adh.visibilityMgr.GetName() == elasticsearch.PersistenceName {
412+
// TODO (rodrigozhou): Remove condition `indexName == ""`.
413+
// If indexName == "", then calling addSearchAttributesElasticsearch will
414+
// register the search attributes in the cluster metadata if ES is up or if
415+
// `skip-schema-update` is set. This is for backward compatibility using
416+
// standard visibility.
417+
if adh.visibilityMgr.GetName() == elasticsearch.PersistenceName || indexName == "" {
408418
err = adh.removeSearchAttributesElasticsearch(ctx, request, indexName)
409419
} else {
410420
err = adh.removeSearchAttributesSQL(ctx, request)
@@ -505,7 +515,12 @@ func (adh *AdminHandler) GetSearchAttributes(
505515
return nil, serviceerror.NewUnavailable(fmt.Sprintf(errUnableToGetSearchAttributesMessage, err))
506516
}
507517

508-
if adh.visibilityMgr.GetName() == elasticsearch.PersistenceName {
518+
// TODO (rodrigozhou): Remove condition `indexName == ""`.
519+
// If indexName == "", then calling addSearchAttributesElasticsearch will
520+
// register the search attributes in the cluster metadata if ES is up or if
521+
// `skip-schema-update` is set. This is for backward compatibility using
522+
// standard visibility.
523+
if adh.visibilityMgr.GetName() == elasticsearch.PersistenceName || indexName == "" {
509524
return adh.getSearchAttributesElasticsearch(ctx, indexName, searchAttributes)
510525
}
511526
return adh.getSearchAttributesSQL(request, searchAttributes)

‎service/frontend/operator_handler.go

+18-3
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,12 @@ func (h *OperatorHandlerImpl) AddSearchAttributes(
192192
}
193193
}
194194

195-
if h.visibilityMgr.GetName() == elasticsearch.PersistenceName {
195+
// TODO (rodrigozhou): Remove condition `indexName == ""`.
196+
// If indexName == "", then calling addSearchAttributesElasticsearch will
197+
// register the search attributes in the cluster metadata if ES is up or if
198+
// `skip-schema-update` is set. This is for backward compatibility using
199+
// standard visibility.
200+
if h.visibilityMgr.GetName() == elasticsearch.PersistenceName || indexName == "" {
196201
err = h.addSearchAttributesElasticsearch(ctx, request, indexName)
197202
} else {
198203
err = h.addSearchAttributesSQL(ctx, request, currentSearchAttributes)
@@ -325,7 +330,12 @@ func (h *OperatorHandlerImpl) RemoveSearchAttributes(
325330

326331
var err error
327332
indexName := h.visibilityMgr.GetIndexName()
328-
if h.visibilityMgr.GetName() == elasticsearch.PersistenceName {
333+
// TODO (rodrigozhou): Remove condition `indexName == ""`.
334+
// If indexName == "", then calling addSearchAttributesElasticsearch will
335+
// register the search attributes in the cluster metadata if ES is up or if
336+
// `skip-schema-update` is set. This is for backward compatibility using
337+
// standard visibility.
338+
if h.visibilityMgr.GetName() == elasticsearch.PersistenceName || indexName == "" {
329339
err = h.removeSearchAttributesElasticsearch(ctx, request, indexName)
330340
} else {
331341
err = h.removeSearchAttributesSQL(ctx, request)
@@ -425,7 +435,12 @@ func (h *OperatorHandlerImpl) ListSearchAttributes(
425435
)
426436
}
427437

428-
if h.visibilityMgr.GetName() == elasticsearch.PersistenceName {
438+
// TODO (rodrigozhou): Remove condition `indexName == ""`.
439+
// If indexName == "", then calling addSearchAttributesElasticsearch will
440+
// register the search attributes in the cluster metadata if ES is up or if
441+
// `skip-schema-update` is set. This is for backward compatibility using
442+
// standard visibility.
443+
if h.visibilityMgr.GetName() == elasticsearch.PersistenceName || indexName == "" {
429444
return h.listSearchAttributesElasticsearch(ctx, indexName, searchAttributes)
430445
}
431446
return h.listSearchAttributesSQL(request, searchAttributes)

‎service/history/fx.go

+8-5
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ import (
4242
"go.temporal.io/server/common/metrics"
4343
"go.temporal.io/server/common/namespace"
4444
persistenceClient "go.temporal.io/server/common/persistence/client"
45+
"go.temporal.io/server/common/persistence/sql/sqlplugin/mysql"
46+
"go.temporal.io/server/common/persistence/sql/sqlplugin/postgresql"
47+
"go.temporal.io/server/common/persistence/sql/sqlplugin/sqlite"
4548
"go.temporal.io/server/common/persistence/visibility"
4649
"go.temporal.io/server/common/persistence/visibility/manager"
4750
"go.temporal.io/server/common/persistence/visibility/store/elasticsearch"
@@ -165,11 +168,11 @@ func ConfigProvider(
165168
indexName := ""
166169
if persistenceConfig.StandardVisibilityConfigExist() {
167170
storeConfig := persistenceConfig.DataStores[persistenceConfig.VisibilityStore]
168-
switch {
169-
case storeConfig.Cassandra != nil:
170-
indexName = storeConfig.Cassandra.Keyspace
171-
case storeConfig.SQL != nil:
172-
indexName = storeConfig.SQL.DatabaseName
171+
if storeConfig.SQL != nil {
172+
switch storeConfig.SQL.PluginName {
173+
case mysql.PluginNameV8, postgresql.PluginNameV12, sqlite.PluginName:
174+
indexName = storeConfig.SQL.DatabaseName
175+
}
173176
}
174177
} else if persistenceConfig.AdvancedVisibilityConfigExist() {
175178
indexName = esConfig.GetVisibilityIndex()

‎tests/test_cluster.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,11 @@ func NewCluster(options *TestClusterConfig, logger log.Logger) (*TestCluster, er
177177
}
178178
} else {
179179
storeConfig := pConfig.DataStores[pConfig.VisibilityStore]
180-
switch {
181-
case storeConfig.Cassandra != nil:
182-
indexName = storeConfig.Cassandra.Keyspace
183-
case storeConfig.SQL != nil:
184-
indexName = storeConfig.SQL.DatabaseName
180+
if storeConfig.SQL != nil {
181+
switch storeConfig.SQL.PluginName {
182+
case mysql.PluginNameV8, postgresql.PluginNameV12, sqlite.PluginName:
183+
indexName = storeConfig.SQL.DatabaseName
184+
}
185185
}
186186
}
187187

0 commit comments

Comments
 (0)