services/horizon/internal/db2/history: Insert and query rows from history lookup tables with one query

[tamirms]

PR Checklist

PR Structure

• This PR has reasonably narrow scope (if not, break it down into smaller PRs).
• This PR avoids mixing refactoring changes with feature changes (split into two PRs
  otherwise).
• This PR's title starts with name of package that is most changed in the PR, ex.
  services/friendbot, or all or doc if the changes are broad or impact many
  packages.

Thoroughness

• This PR adds tests for the most critical parts of the new functionality or fixes.
• I've updated any docs (developer docs, .md
  files, etc... affected by this change). Take a look in the docs folder for a given service,
  like this one.

Release planning

• I've updated the relevant CHANGELOG (here for Horizon) if
  needed with deprecations, added features, breaking changes, and DB schema changes.
• I've decided if this PR requires a new major/minor version according to
  semver, or if it's mainly a patch change. The PR is targeted at the next
  release branch if it's not a patch change.

What

fix #4864

Currently, the loaders for the history lookup tables attempt to get or create history ids by issuing up to 3 SQL statements. First, we query for any existing rows. Then we insert the rows that are not present. Finally, we query the rows that were just inserted.

In #4864 we discussed that if we included a RETURNING clause to the insert statements we could obtain all the rows both present and inserted in a single query.

However, @urvisavla pointed out:

The RETURNING clause in an INSERT statement only returns the newly inserted rows. To return all rows, including existing ones, we can force an update using the ON CONFLICT UPDATE SET... clause. However this additional update will likely have a negative impact on performance.

So that means the RETURNING clause could reduce the number of SQL statements from 3 to 2 but we would still need to do a SQL statement to find all the pre-existing rows.

But, it turns out we can use a UNION clause to combine the INSERT ... RETURNING * statement with another query which just selects all the pre-existing rows. This technique is described in more detail here:

https://hakibenita.com/postgresql-get-or-create

Known limitations

Unfortunately when deploying this PR to staging I did not observe any significant improvements to the "Ingestion Loaders Duration" metric. Also, with this change we lose insight into the number of rows which were inserted vs queried. Nevertheless, I do think it's still worth including this change as it reduces code complexity

[tamirms]

@urvisavla can you take another look? I believe I've addressed all the feedback

[tamirms]

Actually it turns out there's a possible race condition in the get or create query where the query can return 0 rows if there are 2 transactions attempting to insert the same data concurrently. See https://stackoverflow.com/a/15950324 for more details. I will need to update this PR

[tamirms]

@urvisavla now it's ready for review again. I fixed the race condition but unfortunately we cannot do both insertion and querying within a single sql statement

[urvisavla]

So we're essentially removing the initial select, updating the insert to return the inserted rows, and then adding a select to fetch only the remaining rows, correct?

Also, I'm assuming there's no need to check the performance of the change on staging since we're no longer combining the queries, right?

Changes look good to me. Left a couple of comments for consideration. Also the description needs update.

[tamirms]

@urvisavla

So we're essentially removing the initial select, updating the insert to return the inserted rows, and then adding a select to fetch only the remaining rows, correct?

yes, that's correct

Also, I'm assuming there's no need to check the performance of the change on staging since we're no longer combining the queries, right?

I'm still checking the performance of staging to make sure there are no regressions

I also updated the PR to reduce code duplication between all 4 loaders. PTAL, thanks!