Fix update recency issue with expanded test coverage #63
Conversation
arik-so
force-pushed
the
2023/10/ordering_fix
branch
from
d02d1bc to
54a0d67
Compare
arik-so
changed the title
| @@ -143,7 +143,7 @@ pub(crate) fn db_index_creation_query() -> &'static str { | |||
| CREATE INDEX IF NOT EXISTS channel_updates_scid_dir_seen_desc_with_id ON channel_updates(short_channel_id ASC, direction ASC, seen DESC) INCLUDE (id); | |||
| CREATE UNIQUE INDEX IF NOT EXISTS channel_updates_key ON channel_updates (short_channel_id, direction, timestamp); | |||
| CREATE INDEX IF NOT EXISTS channel_updates_seen ON channel_updates(seen); | |||
| CREATE INDEX IF NOT EXISTS channel_updates_timestamp_desc ON channel_updates(timestamp DESC); | |||
There was a problem hiding this comment.
Did you check that this is now unused?
There was a problem hiding this comment.
I only tested the specific query that I now changed, but prepending short_channel_id ASC does make it no longer use that index.
There was a problem hiding this comment.
Right, my question was if any other queries use that index.
There was a problem hiding this comment.
The other three channel_update queries don't seem to be using it with my local test database, but my postgres seems to occasionally select indices distinct from yours, so you may wanna double-check your instance, too
There was a problem hiding this comment.
Mine doesn't seem to either.
arik-so
force-pushed
the
2023/10/ordering_fix
branch
from
248b8b1 to
0aff71f
Compare
| @@ -111,7 +111,7 @@ impl<L: Deref> GossipPersister<L> where L::Target: Logger { | |||
| } | |||
|
|
|||
| match &gossip_message { | |||
| GossipMessage::ChannelAnnouncement(announcement) => { | |||
| GossipMessage::ChannelAnnouncement(announcement, _) => { | |||
There was a problem hiding this comment.
Why add the override field if we can't use it?
There was a problem hiding this comment.
I haven't added the unit test for an old announcement yet that was gonna use it. I can do that now to make sure this does end up finding some use immediately.
There was a problem hiding this comment.
That's alright, I just feel weird adding it freestanding but if you already have code queued up to use it we'll survive.
|
This should have fixed #61, but please let us know if you continue to see issues. |
No description provided.