Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A KeyValue stream's max messages per subject isn't being respected after updating the stream's max messages per subject to be smaller than before #4445

Closed
anthonyjacques20 opened this issue Aug 29, 2023 · 2 comments · Fixed by #4446
Assignees
Labels
defect Suspected defect such as a bug or regression

Comments

@anthonyjacques20
Copy link

What version were you using?

2.9.21

What environment was the server running in?

Mac M1

Is this defect reproducible?

Yes, this is reproducible. Run a nats server with jetstream enabled and then run the following test:

package history_test

import (
	"encoding/json"
	"strconv"
	"testing"

	"github.com/nats-io/nats.go"
	"github.com/stretchr/testify/require"
)

func TestHistory(t *testing.T) {
	// Start server with `nats-server --js` to make sure jetstream is turned on
	nc, err := nats.Connect(nats.DefaultURL)
	require.Nil(t, err)

	// Create jetstream and KeyValue
	js, err := nc.JetStream()
	require.Nil(t, err)
	bucketName := "HistoryTest"
	kv, err := js.CreateKeyValue(&nats.KeyValueConfig{
		Bucket:      bucketName,
		Description: "Test for number of historical values",
		History:     10,
	})
	require.Nil(t, err)
	key := "mykey"

	// Write to the same key 50 times
	for j := 0; j < 50; j++ {
		value := strconv.Itoa(j)
		marshalledBytes, err := json.Marshal(value)
		require.Nil(t, err)
		_, err = kv.Put(key, marshalledBytes)
		require.Nil(t, err)
	}
	// Verify we have the correct amount of history
	info, err := js.StreamInfo("KV_" + bucketName)
	require.Nil(t, err)
	histories, err := kv.History(key)
	require.Nil(t, err)
	require.Len(t, histories, int(info.Config.MaxMsgsPerSubject))

	// Update the stream to store half the amount of history as before
	newConfig := info.Config
	newConfig.MaxMsgsPerSubject = newConfig.MaxMsgsPerSubject / 2
	info, err = js.UpdateStream(&newConfig)
	require.Nil(t, err)

	// Write 50 more values
	for j := 0; j < 50; j++ {
		value := strconv.Itoa(50 + j)
		marshalledBytes, err := json.Marshal(value)
		require.Nil(t, err)
		_, err = kv.Put(key, marshalledBytes)
		require.Nil(t, err)
	}
	// Verify that the history has the updated number of max messages per subject
	histories, err = kv.History(key)
	require.Nil(t, err)
	require.Len(t, histories, int(info.Config.MaxMsgsPerSubject)) // We fail here...
}

Given the capability you are leveraging, describe your expectation?

My expectation is that after updating the Max Messages Per Subject for the Stream, the corresponding KeyValue should contain the updated amount of history

Given the expectation, what is the defect you are observing?

After updating the stream associated with the KV, it is storing more history than it should be storing.

@anthonyjacques20 anthonyjacques20 added the defect Suspected defect such as a bug or regression label Aug 29, 2023
@ripienaar
Copy link
Contributor

ripienaar commented Aug 29, 2023

Tried to reproduce this:

$ nats kv add X --replicas 3 --history 10

$ for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
do
nats kv put X y $i
done

$ nats kv history X y
#...shows 10 history

$ nats s edit KV_X --max-msgs-per-subject 5 -f
$ nats kv history X y
#...shows 5 history

$ for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
do
nats kv put X y $i
done

$ nats kv history X y
#...shows 20 history

So can confirm this also affects 2.10

@derekcollison
Copy link
Member

Server issue, will move.

@derekcollison derekcollison transferred this issue from nats-io/nats.go Aug 29, 2023
@derekcollison derekcollison self-assigned this Aug 29, 2023
derekcollison added a commit that referenced this issue Aug 29, 2023
We were not recalculating first correctly since we were not considering
seq < mb.first.seq.

Signed-off-by: Derek Collison <derek@nats.io>

Resolves #4445
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect Suspected defect such as a bug or regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants