Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions common/proto/client.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions common/proto/client.proto
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,8 @@ enum Status {
UNEXPECTED_VERSION_ID = 2;
// The session that the put request referred to is not alive
SESSION_DOES_NOT_EXIST = 3;
// The request contains an invalid argument
INVALID_ARGUMENT = 4;
}

message CreateSessionRequest {
Expand Down
16 changes: 10 additions & 6 deletions common/proto/replication.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions common/proto/replication.proto
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ service OxiaLogReplication {
enum Feature {
FEATURE_UNKNOWN = 0;
FEATURE_DB_CHECKSUM = 1;
FEATURE_SECONDARY_INDEX_NAME_VALIDATION = 2;
}

message GetInfoRequest {
Expand Down
5 changes: 3 additions & 2 deletions oxia/options_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ func (u *useIndex) applyGet(opts *getOptions) {
opts.secondaryIndexName = &u.indexName
}

// UseIndex let the users specify a different index to follow for the
// Note: The returned list will contain they primary keys of the records.
// UseIndex selects the secondary index used to interpret keys and key ranges
// in Get, List, and RangeScan operations. Index names must not contain '/'.
// Returned record keys remain the corresponding primary keys.
func UseIndex(indexName string) ListOption {
return &useIndex{indexName}
}
Expand Down
18 changes: 14 additions & 4 deletions oxia/options_put.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@

package oxia

import "github.com/pkg/errors"
import (
"strings"

"github.com/pkg/errors"
)

type putOptions struct {
baseOptions
Expand All @@ -35,6 +39,13 @@ func newPutOptions(opts []PutOption) (*putOptions, error) {
opt.applyPut(putOpts)
}

for _, secondaryIndex := range putOpts.secondaryIndexes {
if strings.IndexByte(secondaryIndex.indexName, '/') >= 0 {
return nil, errors.Wrapf(ErrInvalidOptions,
"secondary index name %q must not contain '/'", secondaryIndex.indexName)
}
}

if len(putOpts.sequenceKeysDeltas) > 0 {
if putOpts.partitionKey == nil {
return nil, errors.Wrap(ErrInvalidOptions, "usage of sequential keys requires PartitionKey() to be set")
Expand Down Expand Up @@ -106,9 +117,8 @@ func (s *secondaryIdxOption) applyPut(opts *putOptions) {
opts.secondaryIndexes = append(opts.secondaryIndexes, s)
}

// SecondaryIndex let the users specify additional keys to index the record
// Index names are arbitrary strings and can be used in `List` and
// `RangeScan` requests.
// SecondaryIndex lets users specify additional keys to index the record.
// Index names must not contain '/' and can be used in [UseIndex] options.
// Secondary keys are not required to be unique.
// Multiple secondary indexes can be passed on the same record, even
// reusing multiple times the same indexName.
Expand Down
39 changes: 39 additions & 0 deletions oxia/options_put_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright 2023-2026 The Oxia Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package oxia

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/oxia-db/oxia/common/proto"
)

func TestSecondaryIndexNameValidation(t *testing.T) {
_, err := newPutOptions([]PutOption{SecondaryIndex("tenant/users", "email")})
require.Error(t, err)
assert.ErrorIs(t, err, ErrInvalidOptions)
assert.ErrorContains(t, err, "must not contain '/'")

_, err = newPutOptions([]PutOption{SecondaryIndex("tenant", "users/email")})
assert.NoError(t, err)
}

func TestInvalidArgumentStatus(t *testing.T) {
result := toPutResult("key", &proto.PutResponse{Status: proto.Status_INVALID_ARGUMENT})
assert.ErrorIs(t, result.Err, ErrInvalidOptions)
}
2 changes: 2 additions & 0 deletions oxia/proto_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ func toError(status proto.Status) error {
return ErrUnexpectedVersionId
case proto.Status_KEY_NOT_FOUND:
return ErrKeyNotFound
case proto.Status_INVALID_ARGUMENT:
return ErrInvalidOptions
default:
return ErrUnknownStatus
}
Expand Down
5 changes: 5 additions & 0 deletions oxiad/common/feature/feature.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,13 @@ import (
"github.com/oxia-db/oxia/common/proto"
)

type Checker interface {
IsFeatureEnabled(feature proto.Feature) bool
}

func SupportedFeatures() []proto.Feature {
return []proto.Feature{
proto.Feature_FEATURE_DB_CHECKSUM,
proto.Feature_FEATURE_SECONDARY_INDEX_NAME_VALIDATION,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,26 @@ func TestNegotiate_MixedVersions_RollingUpgrade(t *testing.T) {
assert.Contains(t, result, proto.Feature_FEATURE_DB_CHECKSUM, "feature should be enabled after all nodes are upgraded")
}

func TestNegotiate_SecondaryIndexValidationRequiresFullUpgrade(t *testing.T) {
currentFeatures := []proto.Feature{
proto.Feature_FEATURE_DB_CHECKSUM,
proto.Feature_FEATURE_SECONDARY_INDEX_NAME_VALIDATION,
}
nodeFeatures := map[string][]proto.Feature{
"new-node-1": currentFeatures,
"new-node-2": currentFeatures,
"old-node": {proto.Feature_FEATURE_DB_CHECKSUM},
}

result := negotiate(nodeFeatures, 3)
assert.Contains(t, result, proto.Feature_FEATURE_DB_CHECKSUM)
assert.NotContains(t, result, proto.Feature_FEATURE_SECONDARY_INDEX_NAME_VALIDATION)

nodeFeatures["old-node"] = currentFeatures
result = negotiate(nodeFeatures, 3)
assert.Contains(t, result, proto.Feature_FEATURE_SECONDARY_INDEX_NAME_VALIDATION)
}

func TestWaitForMajority_Success(t *testing.T) {
e := &Election{}
server1 := testDataServer("server1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -999,8 +999,8 @@ func TestController_FeatureNegotiation_AllNodesSupport(t *testing.T) {
rpc.GetNode(s2).ExpectNewTermRequest(t, shard, 2, true)
rpc.GetNode(s3).ExpectNewTermRequest(t, shard, 2, true)

// Verify BecomeLeader includes the DB Checksum feature
rpc.GetNode(s1).ExpectBecomeLeaderRequestWithFeatures(t, shard, 2, 3, []proto.Feature{proto.Feature_FEATURE_DB_CHECKSUM})
// Verify BecomeLeader includes all features supported by the ensemble.
rpc.GetNode(s1).ExpectBecomeLeaderRequestWithFeatures(t, shard, 2, 3, feature.SupportedFeatures())

assert.Eventually(t, func() bool {
return shardStatus(metadata, constant.DefaultNamespace, shard) == proto.ShardStatusSteadyState
Expand Down
22 changes: 22 additions & 0 deletions oxiad/dataserver/controller/lead/secondary_indexes.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/oxia-db/oxia/common/compare"
"github.com/oxia-db/oxia/common/constant"
"github.com/oxia-db/oxia/oxiad/common/feature"
"github.com/oxia-db/oxia/oxiad/dataserver/database"
"github.com/oxia-db/oxia/oxiad/dataserver/database/kvstore"

Expand All @@ -34,6 +35,14 @@ const secondaryIdxKeyPrefix = constant.InternalKeyPrefix + "idx"

type wrapperUpdateCallback struct{}

func (wrapperUpdateCallback) ValidatePut(req *proto.PutRequest, features feature.Checker) proto.Status {
if status := sessionManagerUpdateOperationCallback.ValidatePut(req, features); status != proto.Status_OK {
return status
}

return secondaryIndexesUpdateCallback.ValidatePut(req, features)
}

func (wrapperUpdateCallback) OnDeleteWithEntry(batch kvstore.WriteBatch, notifications *database.Notifications, key string, value *proto.StorageEntry) error {
// First update the session
if err := sessionManagerUpdateOperationCallback.OnDeleteWithEntry(batch, notifications, key, value); err != nil {
Expand Down Expand Up @@ -81,6 +90,19 @@ type secondaryIndexesUpdateCallbackS struct{}

var secondaryIndexesUpdateCallback database.UpdateOperationCallback = &secondaryIndexesUpdateCallbackS{}

func (secondaryIndexesUpdateCallbackS) ValidatePut(request *proto.PutRequest, features feature.Checker) proto.Status {
if !features.IsFeatureEnabled(proto.Feature_FEATURE_SECONDARY_INDEX_NAME_VALIDATION) {
return proto.Status_OK
}

for _, secondaryIndex := range request.SecondaryIndexes {
if strings.IndexByte(secondaryIndex.GetIndexName(), '/') >= 0 {
return proto.Status_INVALID_ARGUMENT
}
}
return proto.Status_OK
}

func (secondaryIndexesUpdateCallbackS) OnPut(batch kvstore.WriteBatch, _ *database.Notifications, request *proto.PutRequest, existingEntry *proto.StorageEntry) (proto.Status, error) {
if existingEntry != nil {
// TODO: We might want to check if there are indexes that did not change
Expand Down
64 changes: 64 additions & 0 deletions oxiad/dataserver/controller/lead/secondary_indexes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,77 @@ import (
"github.com/oxia-db/oxia/oxiad/dataserver/option"

"github.com/oxia-db/oxia/common/rpc"
"github.com/oxia-db/oxia/oxiad/common/feature"
"github.com/oxia-db/oxia/oxiad/dataserver/database/kvstore"

"github.com/oxia-db/oxia/common/constant"

"github.com/oxia-db/oxia/common/proto"
)

func TestSecondaryIndexNameValidation(t *testing.T) {
tests := []struct {
name string
validationEnabled bool
secondaryIndexes []*proto.SecondaryIndex
expected proto.Status
}{
{
name: "validation disabled",
secondaryIndexes: []*proto.SecondaryIndex{{
IndexName: "tenant/users",
}},
expected: proto.Status_OK,
},
{name: "no indexes", validationEnabled: true, expected: proto.Status_OK},
{
name: "valid index name",
validationEnabled: true,
secondaryIndexes: []*proto.SecondaryIndex{{
IndexName: "tenant",
SecondaryKey: "users/email",
}},
expected: proto.Status_OK,
},
{
name: "invalid index name",
validationEnabled: true,
secondaryIndexes: []*proto.SecondaryIndex{{
IndexName: "tenant/users",
SecondaryKey: "email",
}},
expected: proto.Status_INVALID_ARGUMENT,
},
{
name: "invalid later index name",
validationEnabled: true,
secondaryIndexes: []*proto.SecondaryIndex{
{IndexName: "tenant", SecondaryKey: "email"},
{IndexName: "tenant/users", SecondaryKey: "email"},
},
expected: proto.Status_INVALID_ARGUMENT,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
request := &proto.PutRequest{SecondaryIndexes: tt.secondaryIndexes}
features := testFeatureChecker{secondaryIndexNameValidation: tt.validationEnabled}
assert.Equal(t, tt.expected, WrapperUpdateOperationCallback.ValidatePut(request, features))
})
}
}

type testFeatureChecker struct {
secondaryIndexNameValidation bool
}

func (f testFeatureChecker) IsFeatureEnabled(candidate proto.Feature) bool {
return f.secondaryIndexNameValidation && candidate == proto.Feature_FEATURE_SECONDARY_INDEX_NAME_VALIDATION
}

var _ feature.Checker = testFeatureChecker{}

func TestSecondaryIndices_List(t *testing.T) {
var shard int64 = 1

Expand Down
5 changes: 5 additions & 0 deletions oxiad/dataserver/controller/lead/session_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (

"github.com/pkg/errors"

"github.com/oxia-db/oxia/oxiad/common/feature"
"github.com/oxia-db/oxia/oxiad/dataserver/database"
"github.com/oxia-db/oxia/oxiad/dataserver/database/kvstore"

Expand Down Expand Up @@ -388,6 +389,10 @@ type sessionManagerUpdateOperationCallbackS struct{}

var sessionManagerUpdateOperationCallback database.UpdateOperationCallback = &sessionManagerUpdateOperationCallbackS{}

func (*sessionManagerUpdateOperationCallbackS) ValidatePut(*proto.PutRequest, feature.Checker) proto.Status {
return proto.Status_OK
}

func (*sessionManagerUpdateOperationCallbackS) OnPutWithinSession(batch kvstore.WriteBatch, notification *database.Notifications, request *proto.PutRequest, existingEntry *proto.StorageEntry) (proto.Status, error) {
var _, closer, err = batch.Get(SessionKey(SessionId(*request.SessionId)))
if err != nil {
Expand Down
Loading
Loading