Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
KV: changing discard policy would fail on srv upgrade
This is related to #900 that introduced the change of discard policy if connecting to a server v2.7.2+. The CreateKeyValue() API calls AddStream(). If the stream already exists and the configuration is identical, the call succeeds (idempotent). However, since we now try to set the discard policy to "new" when connecting to a server v2.7.2+, the call will fail if the KV store already existed (CreateKeyValue() was called with a v2.7.1 server and stream was created with DiscardOld). The approach here is that if AddStream() fails with an "already in use" error, then we will lookup the stream info, and if we detect that the configuration is same (except for the discard policy), then we call UpdateStream() with the new discard policy. The problematic part is that the client side configuration does not set some of the fields (or their value is 0), but then the server sets either some defaults (like Duplicates set to 2min) or replaces 0 to -1. So the info we get back and the config we have need to be tweaked before being compared. This is really hacky and prone to break if server were to change some defaults. Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
- Loading branch information
1 parent
e483e46
commit 373b454
Showing
3 changed files
with
104 additions
and
44 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 |
---|---|---|
@@ -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