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

Fixed a bug that could lead to perceived message loss under JetStream. #2490

Merged
merged 2 commits into from
Sep 7, 2021

Conversation

derekcollison
Copy link
Member

Under load and pressure from concurrent publishing and consuming with multiple consumers the filestore would return a partial or no cache error to the upper layers. For consumers this could result in us skipping a stream sequence when we should not.

This change stabilizes the filestore and removes the flush state for msg blocks. I also found some bugs that did not track last sequence properly after snapshots / restore.

Also increased timeout under Travis for the Go tests. We were bumping up against the 10m default.

Signed-off-by: Derek Collison [email protected]

/cc @nats-io/core

Under load and pressure from concurrent publishing and consuming with multiple consumers the filestore would
return a partial or no cache error to the upper layers. For consumers this could result in us skipping a stream sequence when we should not.

This change stabilizes the filestore and removes the flush state for msg blocks. I also found some bugs that did not track last sequence properly
after snapshots / restore.

Signed-off-by: Derek Collison <[email protected]>
@anjmao
Copy link

anjmao commented Sep 6, 2021

Hi @derekcollison

Looks like this bug #2488 is related to your fixes.

@derekcollison
Copy link
Member Author

Unclear if they are related tbh.

@tigrato
Copy link

tigrato commented Sep 6, 2021

Unclear if they are related tbh.

Not sure if it helps but my JetStream was having the same issues described in #2488 and once I switched from file storage to memory storage it improved a lot and no issues until now.

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, but I believe that there are left over debug print statements.

server/filestore.go Outdated Show resolved Hide resolved
Signed-off-by: Derek Collison <[email protected]>
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

@derekcollison derekcollison merged commit 963b0c4 into main Sep 7, 2021
@derekcollison derekcollison deleted the fs-stable branch September 7, 2021 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants