Skip to content

streaming: fix snapshot cache bug#9772

Merged
dnephin merged 2 commits into
masterfrom
streamin-fix-bad-cached-snapshot
Feb 16, 2021
Merged

streaming: fix snapshot cache bug#9772
dnephin merged 2 commits into
masterfrom
streamin-fix-bad-cached-snapshot

Conversation

@dnephin
Copy link
Copy Markdown
Contributor

@dnephin dnephin commented Feb 16, 2021

Previously a snapshot created as part of a resume-stream request could have incorrectly cached the newSnapshotToFollow event. When the next subscribe request was received with Index=0 the returned snapshot would cause clients to error because they received an unexpected framing event.

Fixed the bug by saving a snapshot without the framing event, then creating a new snapshot to prepend the framing event.

when the cache entry is created from resuming a stream.
@dnephin dnephin added type/bug Feature does not function as expected theme/streaming Related to Streaming connections between server and client labels Feb 16, 2021
@hashicorp-ci
Copy link
Copy Markdown
Contributor

🤔 Double check that this PR does not require a changelog entry in the .changelog directory. Reference

Previously a snapshot created as part of a resumse-stream request could have incorrectly
cached the newSnapshotToFollow event. This would cause clients to error because they
received an unexpected framing event.
@dnephin dnephin force-pushed the streamin-fix-bad-cached-snapshot branch from be3c23f to ba3a1b9 Compare February 16, 2021 17:52
@vercel vercel Bot temporarily deployed to Preview – consul-ui-staging February 16, 2021 17:52 Inactive
@vercel vercel Bot temporarily deployed to Preview – consul February 16, 2021 17:52 Inactive
Copy link
Copy Markdown
Contributor

@banks banks left a comment

Choose a reason for hiding this comment

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

Thanks LGTM!

Copy link
Copy Markdown
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

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

LGTM

@dnephin dnephin merged commit 363d738 into master Feb 16, 2021
@dnephin dnephin deleted the streamin-fix-bad-cached-snapshot branch February 16, 2021 20:28
@hashicorp-ci
Copy link
Copy Markdown
Contributor

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/328108.

@hashicorp-ci
Copy link
Copy Markdown
Contributor

🍒✅ Cherry pick of commit 363d738 onto release/1.9.x succeeded!

hashicorp-ci pushed a commit that referenced this pull request Feb 16, 2021
dizzyup pushed a commit that referenced this pull request Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

theme/streaming Related to Streaming connections between server and client type/bug Feature does not function as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants