Improve cursor-based batched background migration sub-batch queries

Cursor-based BBMs build queries of the form

select * from <table>
where (<start_cursor_cols>) >= (start_cursor)
and (<end_cursor_cols>) <= (end_cursor)
and (<start_cursor_cols>) > (keyset_prev_end_cursor);

If the cursor is a single column, this isn't a problem - postgres collapses to the tightest bound. But if the cursor is multicolumn, postgres can't optimize and we re-read data from the index starting at the beginning of the batch.

Fix the query text so that it only has a single lower and upper bound.

The following discussion from !235210 (merged) should be addressed:

  • @stomlinson started a discussion: (+2 comments)

    There's a bit of an unfortunate query pattern here, and it's shared with all cursor-based migrations, but I don't think we should fix it in this MR.

    underlying_relation is

    select * from <table> where (<start_cursor_cols>) >= (start_cursor) and (<end_cursor_cols>) <= (end_cursor);

    so when the keyset sub-batch iteration happens, it rebuilds the query for each batch as

    select * from <table> where (<start_cursor_cols>) >= (start_cursor) and (<end_cursor_cols>) <= (end_cursor) and (<start_cursor_cols>) > (keyset_prev_end_cursor);

    This causes the same "failure to collapse row conditions" problem as we saw with the view - each sub-batch will scan rows from merge_request_diff_commits starting at the beginning of the outer batch.

    The inefficiency is bounded by the batch size so it's not a huge deal, but there might be about a 2x performance improvement available here.

    It's tricky to fix - we could do Gitlab::Pagination::Keyset::Iterator.new(scope: underlying_relation_without_start.order(cursor_columns), cursor: cursor_columns.zip(start_cursor).to_h), but the keyset iterator expects the cursor to sit before the start of the batch, so it would build where (<cursor_cols>) > (cursor) and we would skip 1 row each batch.

    So I think it should get handled in a follow-up.

Edited by 🤖 GitLab Bot 🤖