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

persist: tune "snapshot ... not yet available" info log #18802

Merged
merged 1 commit into from
Apr 17, 2023
Merged

persist: tune "snapshot ... not yet available" info log #18802

merged 1 commit into from
Apr 17, 2023

Conversation

danhhz
Copy link
Contributor

@danhhz danhhz commented Apr 17, 2023

The main questions this log answers are "was something held up because an upper wasn't caught up" and "is such a case still ongoing", which means we can reduce this log line from being printed N times to twice.

While I'm in here, also increase the threshold for switching from debug to info, because this is semi-expected for small intervals when joining sources to tables. Also remove a bit of noise from the line itself by debug printing the Antichain elements.

Motivation

  • This PR refactors existing code.

Tips for reviewer

Discussion in slack: https://materializeinc.slack.com/archives/C01CFKM1QRF/p1679585192134959

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • This PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way) and therefore is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • This PR includes the following user-facing behavior changes:

@danhhz
persist: tune "snapshot ... not yet available" info log
bc8a686 
The main questions this log answers are "was something held up because
an upper wasn't caught up" and "is such a case still ongoing", which
means we can reduce this log line from being printed N times to twice.

While I'm in here, also increase the threshold for switching from debug
to info, because this is semi-expected for small intervals when joining
sources to tables. Also remove a bit of noise from the line itself by
debug printing the Antichain elements.
@danhhz danhhz requested a review from petrosagg April 17, 2023 17:22
@danhhz danhhz requested a review from a team as a code owner April 17, 2023 17:22
petrosagg
petrosagg approved these changes Apr 17, 2023
View reviewed changes
Copy link
Contributor

@petrosagg petrosagg left a comment

Choose a reason for hiding this comment

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

Thank you!

src/persist-client/src/internal/machine.rs
as_of,
upper,
as_of.elements(),
upper.elements(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice touch!

@danhhz
Copy link
Contributor Author

danhhz commented Apr 17, 2023

TFTR!

@danhhz danhhz enabled auto-merge April 17, 2023 17:47
@danhhz danhhz merged commit 6641abb into MaterializeInc:main Apr 17, 2023
@danhhz danhhz deleted the persist_listen_log branch April 17, 2023 17:58
@materialize-bot materialize-bot mentioned this pull request Apr 21, 2023
Closed
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrosagg petrosagg petrosagg 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
@danhhz @petrosagg