-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix port script fails when DB has no backfilled events. #8729
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Fix port script fails when DB has no backfilled events. Broke in v1.21.0. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -820,14 +820,12 @@ class Porter(object): | |
"ALTER SEQUENCE events_stream_seq RESTART WITH %s", (next_id,) | ||
) | ||
|
||
txn.execute("SELECT -MIN(stream_ordering) FROM events") | ||
txn.execute("SELECT GREATEST(-MIN(stream_ordering), 1) FROM events") | ||
curr_id = txn.fetchone()[0] | ||
if curr_id: | ||
next_id = curr_id + 1 | ||
txn.execute( | ||
"ALTER SEQUENCE events_backfill_stream_seq RESTART WITH %s", | ||
(next_id,), | ||
) | ||
next_id = curr_id + 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sooo.. if there are no backfilled events, the SELECT will return 1, so we'll set the RESTART value to 2. I mean, it's not the end of the world, but it doesn't sound right. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean, it really doesn't matter? I can change it if you want, but we do the exact same thing from a fresh DB too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we do? fair enough. Thought you might change the GREATEST to 0. I don't feel strongly. |
||
txn.execute( | ||
"ALTER SEQUENCE events_backfill_stream_seq RESTART WITH %s", (next_id,), | ||
) | ||
|
||
return self.postgres_store.db_pool.runInteraction( | ||
"_setup_events_stream_seqs", r | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this does fix things, but what is the purpose of negating the minimum
stream_ordering
here? And wouldn't usingGREATEST
and-
here mean that we almost always end up with 1 as it's greater?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stream_ordering
is negative for backfilled events, so if e.g. the min is -52, thenGREATEST(- -52 1)
is52
, while if there are no backfilled events we may end up with a positive min so we getGREATEST(-1, 1)
which is1
.