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

[Streaming Replication - 3rd] Add LSN type and time conversion to and from ms-since-Y2K #53

Merged
merged 3 commits into from Sep 9, 2022

Conversation

osaxma
Copy link
Contributor

@osaxma osaxma commented Sep 9, 2022

  • Adds Log Sequence Number type -- used widely by the Streaming Replication Messages
  • Adds conversion form and to microseconds since 2000-01-01 (the baseline time used by PostgreSQL in replication messages).

@osaxma osaxma changed the title [Streaming Replication - 3nd] Add LSN type and time conversion to and from ms-since-Y2K [Streaming Replication - 3rd] Add LSN type and time conversion to and from ms-since-Y2K Sep 9, 2022
@@ -0,0 +1,12 @@
const _microsecFromUnixEpochToY2K = 946684800 * 1000000;
Copy link
Owner

Choose a reason for hiding this comment

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

Nitpicking: This seems to be a magic number. Can we get the same value from DateTime.utc([...?]).microsecondsSinceEpoch ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I don't know what I was thinking.. fixed!

}

static int _parseLSNString(String string) {
int upperhalf;
Copy link
Owner

Choose a reason for hiding this comment

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

delete these two

if (halves.length != 2) {
throw Exception('Invalid LSN String was given ($string)');
}
upperhalf = int.parse(halves[0], radix: 16) << 32;
Copy link
Owner

Choose a reason for hiding this comment

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

final upperhalf =...
same with lowerhalf

int lowerhalf;
final halves = string.split('/');
if (halves.length != 2) {
throw Exception('Invalid LSN String was given ($string)');
Copy link
Owner

Choose a reason for hiding this comment

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

Make it FormatException or maybe PostgreSQLException.

// These two numbers are equal but in different formats
//
// see: https://www.postgresql.org/docs/current/datatype-pg-lsn.html
const _lsnStringSample = '16/B374D848';
Copy link
Owner

Choose a reason for hiding this comment

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

I think these could be inlined in the test, with the above comment at the level of the test().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@isoos isoos merged commit 7267372 into isoos:master Sep 9, 2022
@osaxma osaxma deleted the add_lsn_type_and_time_convert branch September 10, 2022 09:31
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.

None yet

2 participants