This commit simplifies insertChanEdgePolicyMig. Much of the logic can be
removed given that this method is only used in the context of the graph
SQL migration.
This should improve the performance of the migration quite a lot since
it removes the extra GetChannelAndNodesBySCID call.
Finally, we make the channel-policy part of the SQL migration idempotent
by adding a migration-only policy insert query which will not error out
if the policy already exists and does not have a timestamp that is newer
than the existing records timestamp. To keep the commit simple, a
insertChanEdgePolicyMig function is added which is basically identical
to the updateChanEdgePolicy function except for the fact that it uses
the newly added query. In the next commit, it will be simplified even
more.
In this commit, we make the channel part of the graph SQL migration
idempotent (retry-safe!). We do this by adding a migration-only channel
insert query that will not error out if a the query is called and a
chanenl with the given scid&version already exists. We also ensure that
errors are not thrown if existing channel features & extra types are
re-added.
There is no need to use the "collect-then-update" pattern for node
insertion during the SQL migration since if we do have any previously
persisted data for the node and happen to re-run the insertion for that
node, the data will be exactly the same. So we can make use of "On
conflict, no nothing" here too.
In this commit, the graph SQL migration is updates so that the node
migration step is retry-safe. This is done by using migration specific
logic & queries that do not use the same node-update-constraint as the
normal node upsert logic. For normal "run-time" logic, we always expect
a node update to have a newer timestamp than any previously stored one.
But for the migration, we will only ever be dealing with a single
announcement for a given node & to make things retry-safe, we dont want
the query to error if we re-insert the exact same node.
In preparation for handling retries on the source DB side, we thread
through the `reset` call-backs properly so that we can reset appropriate
variables.
In preparation for making the channel & policy migration logic
idempotent in a step-by-step manner, we add a test here that only tests
the migration of channels _without_ policies so that we can first focus
on just making the channel migration idempotent.
Currently, the graph SQL migration is not retry safe. Meaning that if
the source DB exeutes a retry under the hood, this could result in the
migration failing. In preparation for fixing this, we adust the
migration test accordingly.
This will help us test idempotency later on, but it also ensures that
TestMigrateGraphToSQL is properly testing writes to the
graph_channel_policy_extra_types table.
The first byte of an opaque addr must be one that we dont understand
yet. We do this update in preparation for doing an on-the-fly parse of
persisted opaque addrs to see if they contain addrs that we now support.
For this to work, the first byte cant be 0x01 since this maps to a known
address.
cause we will need to be able to deserialise any persisted OpaqueAddrs
which will have a Payload that we should be able to decode with this
lnwire method to extract any new addresses that we now support.
Co-authored-by: Elle Mouton <elle.mouton@gmail.com>
We may have already persisted node announcements that have multiple DNS
addresses since we may have received them before updating our code to
check for this. So here we just make sure not to send these on to our
peers.
Check that the node ann doesnt contain more than 1 DNS addr.
This will ensure that we now start rejecting new node announcements
with multiple DNS addrs since this check is called in the gossiper
before persisting a node ann to our local graph.
It also validates the DNS fields according to BOLT #7 specs.
In this commit, we remove `AddrLen` as prepration step
before adding DNS address type which will have a var length.
Co-authored-by: Elle Mouton <elle.mouton@gmail.com>
We put this new schema update into the main line and change the
versions of the schema updates which are currently only available
in dev builds. The schemas need to be chronological therefore we
also need to rename the file numbers.
Finally, we update the migrateZombieIndex function to use batch
validation just like was done in the previous commits. Here, we
additionally make sure to validate the entire zombie index entry and not
just the SCID.
As was done in the previous commits for nodes & channels, we update the
migrateClosedSCIDIndex function here so that it validates migrated
entries in batches rather than one-by-one.
As was done in the previous commits for nodes & channels, we update the
migratePruneLog function here so that it validates migrated entries in
batches rather than one-by-one.
Restructue the `migrateChannelsAndPolicies` function so that it does the
validation of migrated channels and policies in batches. So instead of
fetching channel and its policies individually after migrating it, we
wait for a minimum batch size to be reached and then validate a batch of
them together. This lets us make way fewer DB round trips.
Restructue the `migrateNodes` function so that it does the validation of
migrated nodes in batches. So instead of fetching each node individually
after migrating it, we wait for a minimum batch size to be reached and
then validate a batch of nodes together. This lets us make way fewer DB
round trips.
We do this so that this lookup is only done in the situation it is
actually needed. During a migration, we dont need to special case this
AlreadyExists error since we will terminate the transaction either way.
So there is no need for the extra lookup during the migration.
A timing analysis showed that this query was significantly impacting the
performance of the migration when run with a postgres backend.
This commit adds a BenchmarkFindOptimalSQLQueryConfig test in the
graph/db package which runs ForEachNode and ForEachChannel queries
against a local backend using various different values for the sql
QueryConfig struct. This is done to determine good default values to
use for the config options for sqlite vs postgres.
Now that the SQL query config values are configurable, we add some
validation to make sure that the user doesnt set a max batch size that
is larger than the limits for sqlite/postgres that have been determined
by the TestSQLSliceQueries test.
Here, we make the sql query option params (batch size and pagination
size) configurable. The defaults for SQLite vs Postgres are still the
same but will be changed in an upcoming commit.
