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.

Sign up for GitHub

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

Jump to bottom

Fixes issue on restart where mirror config was invalid #3060

Merged
merged 1 commit into from
Apr 20, 2022
Merged

Fixes issue on restart where mirror config was invalid #3060

merged 1 commit into from
Apr 20, 2022

Conversation

matthiashanel
Copy link
Contributor

This issue was introduced by a bug that assigned a de dupe default value
do mirrors.
This is an invalid config that used to be checked on stream create.
Subsequently this logic was moved and then got executed on startup.
Ending up checking mirrors that got the bad default assigned.
Fix is to clear de dupe window for mirrors

Signed-off-by: Matthias Hanel mh@synadia.com

manual restart test.
1st start is without this change
2nd with this

> nats-server -sd ./store -js
[95749] 2022/04/20 13:03:42.591726 [INF] Starting nats-server
[95749] 2022/04/20 13:03:42.591833 [INF]   Version:  2.8.1-beta
[95749] 2022/04/20 13:03:42.591836 [INF]   Git:      [not set]
[95749] 2022/04/20 13:03:42.591842 [INF]   Name:     NDNENZRBTENE453IVC3G2F53VVFZU3U4TWGN7Z5KPHVKXRBFDKB5ODUB
[95749] 2022/04/20 13:03:42.591846 [INF]   Node:     f4skLVsG
[95749] 2022/04/20 13:03:42.591848 [INF]   ID:       NDNENZRBTENE453IVC3G2F53VVFZU3U4TWGN7Z5KPHVKXRBFDKB5ODUB
[95749] 2022/04/20 13:03:42.592195 [INF] Starting JetStream
[95749] 2022/04/20 13:03:42.592508 [INF]     _ ___ _____ ___ _____ ___ ___   _   __  __
[95749] 2022/04/20 13:03:42.592514 [INF]  _ | | __|_   _/ __|_   _| _ \ __| /_\ |  \/  |
[95749] 2022/04/20 13:03:42.592517 [INF] | || | _|  | | \__ \ | | |   / _| / _ \| |\/| |
[95749] 2022/04/20 13:03:42.592519 [INF]  \__/|___| |_| |___/ |_| |_|_\___/_/ \_\_|  |_|
[95749] 2022/04/20 13:03:42.592520 [INF]
[95749] 2022/04/20 13:03:42.592522 [INF]          https://docs.nats.io/jetstream
[95749] 2022/04/20 13:03:42.592523 [INF]
[95749] 2022/04/20 13:03:42.592525 [INF] ---------------- JETSTREAM ----------------
[95749] 2022/04/20 13:03:42.592528 [INF]   Max Memory:      48.00 GB
[95749] 2022/04/20 13:03:42.592531 [INF]   Max Storage:     524.12 GB
[95749] 2022/04/20 13:03:42.592532 [INF]   Store Directory: "store/jetstream"
[95749] 2022/04/20 13:03:42.592534 [INF] -------------------------------------------
goroutine 1 [running]:
runtime/debug.Stack()
	/usr/local/Cellar/go/1.17.7/libexec/src/runtime/debug/stack.go:24 +0x65
runtime/debug.PrintStack()
	/usr/local/Cellar/go/1.17.7/libexec/src/runtime/debug/stack.go:16 +0x19
github.com/nats-io/nats-server/v2/server.(*Server).checkStreamCfg(0xc0001b4000, 0xc0001ee018, 0xc000126b40)
	/Users/matthiashanel/repos/nats-server/server/stream.go:962 +0x8fd
github.com/nats-io/nats-server/v2/server.(*Account).addStreamWithAssignment(0xc000126b40, 0xc0001ee018, 0x0, 0x0)
	/Users/matthiashanel/repos/nats-server/server/stream.go:299 +0x196
github.com/nats-io/nats-server/v2/server.(*Account).addStream(0xc0001ec000, 0xc0001e0170)
	/Users/matthiashanel/repos/nats-server/server/stream.go:277 +0x1d
github.com/nats-io/nats-server/v2/server.(*Account).EnableJetStream(0xc000126b40, 0xc000109890)
	/Users/matthiashanel/repos/nats-server/server/jetstream.go:1193 +0x2ecd
github.com/nats-io/nats-server/v2/server.(*Server).configJetStream(0xc0001b4000, 0xc000126b40)
	/Users/matthiashanel/repos/nats-server/server/jetstream.go:645 +0x9c
github.com/nats-io/nats-server/v2/server.(*Server).enableJetStreamAccounts(0xc000130420)
	/Users/matthiashanel/repos/nats-server/server/jetstream.go:575 +0xb4
github.com/nats-io/nats-server/v2/server.(*Server).enableJetStream(0xc0001b4000, {0xc00000000, 0x83076e2800, {0xc000029f60, 0xf}, {0x0, 0x0}})
	/Users/matthiashanel/repos/nats-server/server/jetstream.go:392 +0x919
github.com/nats-io/nats-server/v2/server.(*Server).EnableJetStream(0xc0001b4000, 0xc00016de38)
	/Users/matthiashanel/repos/nats-server/server/jetstream.go:204 +0x40c
github.com/nats-io/nats-server/v2/server.(*Server).Start(0xc0001b4000)
	/Users/matthiashanel/repos/nats-server/server/server.go:1746 +0xf10
github.com/nats-io/nats-server/v2/server.Run(...)
	/Users/matthiashanel/repos/nats-server/server/service.go:22
main.main()
	/Users/matthiashanel/repos/nats-server/main.go:118 +0x2fa
[95749] 2022/04/20 13:03:42.593124 [WRN]   Error recreating stream "mirror": stream mirrors do not make use of a de-duplication window (10052)
[95749] 2022/04/20 13:03:42.593751 [INF]   Restored 0 messages for stream '$G > stream'
[95749] 2022/04/20 13:03:42.593961 [INF] Listening for client connections on 0.0.0.0:4222
[95749] 2022/04/20 13:03:42.594192 [INF] Server is ready
^C[95749] 2022/04/20 13:07:46.536639 [INF] Initiating Shutdown...
[95749] 2022/04/20 13:07:46.536658 [INF] Initiating JetStream Shutdown...
[95749] 2022/04/20 13:07:46.536761 [INF] JetStream Shutdown
[95749] 2022/04/20 13:07:46.536819 [INF] Server Exiting..
> nats-server -sd ./store -js
[95879] 2022/04/20 13:07:57.008964 [INF] Starting nats-server
[95879] 2022/04/20 13:07:57.009060 [INF]   Version:  2.8.1-beta
[95879] 2022/04/20 13:07:57.009063 [INF]   Git:      [not set]
[95879] 2022/04/20 13:07:57.009065 [INF]   Name:     NAW44HLHTHIRDEBXJRHEJOUL4FOTJ5O4PCR3SEO7Q45CNUI2C5B6LZCG
[95879] 2022/04/20 13:07:57.009070 [INF]   Node:     SE7tDBgf
[95879] 2022/04/20 13:07:57.009072 [INF]   ID:       NAW44HLHTHIRDEBXJRHEJOUL4FOTJ5O4PCR3SEO7Q45CNUI2C5B6LZCG
[95879] 2022/04/20 13:07:57.009432 [INF] Starting JetStream
[95879] 2022/04/20 13:07:57.009739 [INF]     _ ___ _____ ___ _____ ___ ___   _   __  __
[95879] 2022/04/20 13:07:57.009747 [INF]  _ | | __|_   _/ __|_   _| _ \ __| /_\ |  \/  |
[95879] 2022/04/20 13:07:57.009750 [INF] | || | _|  | | \__ \ | | |   / _| / _ \| |\/| |
[95879] 2022/04/20 13:07:57.009752 [INF]  \__/|___| |_| |___/ |_| |_|_\___/_/ \_\_|  |_|
[95879] 2022/04/20 13:07:57.009754 [INF]
[95879] 2022/04/20 13:07:57.009756 [INF]          https://docs.nats.io/jetstream
[95879] 2022/04/20 13:07:57.009758 [INF]
[95879] 2022/04/20 13:07:57.009759 [INF] ---------------- JETSTREAM ----------------
[95879] 2022/04/20 13:07:57.009764 [INF]   Max Memory:      48.00 GB
[95879] 2022/04/20 13:07:57.009766 [INF]   Max Storage:     524.09 GB
[95879] 2022/04/20 13:07:57.009768 [INF]   Store Directory: "store/jetstream"
[95879] 2022/04/20 13:07:57.009770 [INF] -------------------------------------------
[95879] 2022/04/20 13:07:57.011161 [INF]   Restored 0 messages for stream '$G > mirror'
[95879] 2022/04/20 13:07:57.011762 [INF]   Restored 0 messages for stream '$G > stream'
[95879] 2022/04/20 13:07:57.012041 [INF] Listening for client connections on 0.0.0.0:4222
[95879] 2022/04/20 13:07:57.012327 [INF] Server is ready
^C[95879] 2022/04/20 13:08:00.951914 [INF] Initiating Shutdown...
[95879] 2022/04/20 13:08:00.951934 [INF] Initiating JetStream Shutdown...
[95879] 2022/04/20 13:08:00.952030 [INF] JetStream Shutdown
[95879] 2022/04/20 13:08:00.952090 [INF] Server Exiting..

@matthiashanel
Fixes issue on restart where mirror config was invalid
151eaf2 
This issue was introduced by a bug that assigned a de dupe default value
do mirrors.
This is an invalid config that used to be checked on stream create.
Subsequently this logic was moved and then got executed on startup.
Ending up checking mirrors that got the bad default assigned.
Fix is to clear de dupe window for mirrors

Signed-off-by: Matthias Hanel <mh@synadia.com>
kozlovic
kozlovic approved these changes Apr 20, 2022
View reviewed changes
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Do we know where we are setting the default dedup for mirrors, if we are still doing it?

@matthiashanel
Copy link
Contributor Author

@kozlovic

Do we know where we are setting the default dedup for mirrors, if we are still doing it?

No we don't, I actually addressed this, didn't make the connection to this being persisted on disk already.

	if cfg.Duplicates == 0 && cfg.Mirror == nil {
		maxWindow := StreamDefaultDuplicatesWindow
		if lim.Duplicates > 0 && maxWindow > lim.Duplicates {
			maxWindow = lim.Duplicates
		}
		if cfg.MaxAge != 0 && cfg.MaxAge < maxWindow {
			cfg.Duplicates = cfg.MaxAge
		} else {
			cfg.Duplicates = maxWindow
		}
	}

@matthiashanel matthiashanel merged commit 9db2e91 into main Apr 20, 2022
@matthiashanel matthiashanel deleted the fix-mirror-dedupe-onload branch April 20, 2022 17:58
kozlovic added a commit that referenced this pull request Apr 21, 2022
@kozlovic
[FIXED] JetStream: Mirrors would fail to be recovered
363849b 
This is a continuation of PR #3060, but extends to clustering.

Verified with manual test that a mirror created with v2.7.4 has
the duplicates window set and on restart with main would still
complain about use of dedup in cluster mode. The mirror stream
was recovered but showing as R1.
With this fix, a restart of the cluster - with existing data -
will properly recover the stream as an R3 and messages that
were published while in a bad state are synchronized.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Signed-off-by: Matthias Hanel mh@synadia.com
@kozlovic kozlovic mentioned this pull request Apr 21, 2022
Merged
@kozlovic kozlovic mentioned this pull request Jun 29, 2022
Merged
derekcollison added a commit that referenced this pull request Jun 29, 2022
@derekcollison
Disallow duplicate window on mirrors, but accept default in case not …
811c995 
…set, re: #3060

Signed-off-by: Derek Collison <derek@nats.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kozlovic kozlovic kozlovic approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@matthiashanel @kozlovic