Skip to content
This repository was archived by the owner on May 1, 2023. It is now read-only.

Clean up LogicalReplicationListener#45

Merged
cjfinnell merged 2 commits into
masterfrom
cjfinnell/lr-nits
Jan 14, 2022
Merged

Clean up LogicalReplicationListener#45
cjfinnell merged 2 commits into
masterfrom
cjfinnell/lr-nits

Conversation

@cjfinnell

Copy link
Copy Markdown

This is a subset of the changes contained in #37 and is intended to be a more focused/digestible/reviewable change.

@cjfinnell
cjfinnell requested a review from a team as a code owner January 10, 2022 21:47
@cjfinnell
cjfinnell requested review from SudKul and cornerkitten and removed request for a team and SudKul January 10, 2022 21:47
Comment thread lr_listener.go Outdated

@bernielomax bernielomax left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just a couple of questions.

Comment thread lr_listener.go
continue
}

if msg.WalMessage != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should this log/continue also? When are wal messages nil?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

a msg from the db could include either/neither/both WalMessage and ServerHeartbeat - the previous codepath ignored the possible ServerHeartbeat if it found a WalMessage first

Comment thread lr_listener.go Outdated
Comment thread lr_listener.go
} else {
l.logger.Warnf("received WAL with LSN %v, which is less than the current LSN %v", lsn, l.replLSN)
if lsn <= l.replLSN {
l.logger.Errorf("received WAL with LSN %v, which is not greater than last received LSN %v", lsn, l.replLSN)

@bernielomax bernielomax Jan 13, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: The message formatter could be more dry but not essential.

This stored info is not used yet, but we will be using it to confirm if
the target DB is in-sync with the source.
@cjfinnell
cjfinnell merged commit 878cb8f into master Jan 14, 2022
@cjfinnell
cjfinnell deleted the cjfinnell/lr-nits branch January 14, 2022 16:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants