Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #917 from nats-io/kv_create_updgrade_issue
KV: changing discard policy would fail on srv upgrade
- Loading branch information
Showing
5 changed files
with
107 additions
and
46 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ import ( | |
"context" | ||
"errors" | ||
"fmt" | ||
"reflect" | ||
"regexp" | ||
"strconv" | ||
"strings" | ||
|
@@ -326,18 +327,31 @@ func (js *js) CreateKeyValue(cfg *KeyValueConfig) (KeyValue, error) { | |
replicas = 1 | ||
} | ||
|
||
// We will set explicitly some values so that we can do comparison | ||
// if we get an "already in use" error and need to check if it is same. | ||
maxBytes := cfg.MaxBytes | ||
if maxBytes == 0 { | ||
maxBytes = -1 | ||
} | ||
maxMsgSize := cfg.MaxValueSize | ||
if maxMsgSize == 0 { | ||
maxMsgSize = -1 | ||
} | ||
scfg := &StreamConfig{ | ||
Name: fmt.Sprintf(kvBucketNameTmpl, cfg.Bucket), | ||
Description: cfg.Description, | ||
Subjects: []string{fmt.Sprintf(kvSubjectsTmpl, cfg.Bucket)}, | ||
MaxMsgsPerSubject: history, | ||
MaxBytes: cfg.MaxBytes, | ||
MaxBytes: maxBytes, | ||
MaxAge: cfg.TTL, | ||
MaxMsgSize: cfg.MaxValueSize, | ||
MaxMsgSize: maxMsgSize, | ||
Storage: cfg.Storage, | ||
Replicas: replicas, | ||
AllowRollup: true, | ||
DenyDelete: true, | ||
Duplicates: 2 * time.Minute, | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong.
kozlovic
Author
Member
|
||
MaxMsgs: -1, | ||
MaxConsumers: -1, | ||
} | ||
|
||
// If we are at server version 2.7.2 or above use DiscardNew. We can not use DiscardNew for 2.7.1 or below. | ||
|
@@ -346,7 +360,24 @@ func (js *js) CreateKeyValue(cfg *KeyValueConfig) (KeyValue, error) { | |
} | ||
|
||
if _, err := js.AddStream(scfg); err != nil { | ||
return nil, err | ||
// If we have a failure to add, it could be because we have | ||
// a config change if the KV was created against a pre 2.7.2 | ||
// and we are now moving to a v2.7.2+. If that is the case | ||
// and the only difference is the discard policy, then update | ||
// the stream. | ||
if err == ErrStreamNameAlreadyInUse { | ||
if si, _ := js.StreamInfo(scfg.Name); si != nil { | ||
// To compare, make the server's stream info discard | ||
// policy same than ours. | ||
si.Config.Discard = scfg.Discard | ||
if reflect.DeepEqual(&si.Config, scfg) { | ||
_, err = js.UpdateStream(scfg) | ||
} | ||
} | ||
} | ||
if err != nil { | ||
return nil, err | ||
} | ||
} | ||
|
||
kv := &kvs{ | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
// Copyright 2022 The NATS 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 nats | ||
|
||
import ( | ||
"testing" | ||
) | ||
|
||
func TestKeyValueDiscardOldToDiscardNew(t *testing.T) { | ||
s := RunBasicJetStreamServer() | ||
defer shutdownJSServerAndRemoveStorage(t, s) | ||
|
||
nc, js := jsClient(t, s) | ||
defer nc.Close() | ||
|
||
checkDiscard := func(expected DiscardPolicy) KeyValue { | ||
t.Helper() | ||
kv, err := js.CreateKeyValue(&KeyValueConfig{Bucket: "TEST", History: 1}) | ||
if err != nil { | ||
t.Fatalf("Error creating store: %v", err) | ||
} | ||
si, err := js.StreamInfo("KV_TEST") | ||
if err != nil { | ||
t.Fatalf("Error getting stream info: %v", err) | ||
} | ||
if si.Config.Discard != expected { | ||
t.Fatalf("Expected discard policy %v, got %+v", expected, si) | ||
} | ||
return kv | ||
} | ||
|
||
// We are going to go from 2.7.1->2.7.2->2.7.1 and 2.7.2 again. | ||
for i := 0; i < 2; i++ { | ||
// Change the server version in the connection to | ||
// create as-if we were connecting to a v2.7.1 server. | ||
nc.mu.Lock() | ||
nc.info.Version = "2.7.1" | ||
nc.mu.Unlock() | ||
|
||
kv := checkDiscard(DiscardOld) | ||
if i == 0 { | ||
if _, err := kv.PutString("foo", "value"); err != nil { | ||
t.Fatalf("Error adding key: %v", err) | ||
} | ||
} | ||
|
||
// Now change version to 2.7.2 | ||
nc.mu.Lock() | ||
nc.info.Version = "2.7.2" | ||
nc.mu.Unlock() | ||
|
||
kv = checkDiscard(DiscardNew) | ||
// Make sure the key still exists | ||
if e, err := kv.Get("foo"); err != nil || string(e.Value()) != "value" { | ||
t.Fatalf("Error getting key: err=%v e=%+v", err, e) | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@kozlovic this should be the same as
cfg.TTL
whencfg.TTL
is less than 2 minutes, else you get a failure when making TTL less than 2 minutes ofduplicates window can not be larger then max age
, so essentially this introduces a regression where KV TTL is minimum 2 minutes.