Adds urnBasedPagination option to datahub-upgrade RestoreIndices

[nmbryant]

We found that the RestoreIndices job in datahub-upgrade performs poorly with large amounts of data in the SQL database. What we were seeing before this change was that as RestoreIndices ran, each batch would take longer and longer, with the bottleneck being SQL. The reason for this is that using OFFSET causes queries to slowdown as the OFFSET value gets higher.

This solution uses Keyset Pagination by saving the most recent urn and aspect that has been processed. The former method is still part of RestoreIndices, with the flag urnBasedPagination being set to true to use the new keyset pagination method. From our internal testing, we saw full restoration of ~5M records go from 4-5 hours to 40 minutes with linear execution instead of getting slower over time. The downside to this implementation is that currently I haven't added support for multiple threads. However, in most cases this implementation should be must faster than before, since in our example comparison, that was with 32 threads against the new implementation's single thread.

Checklist

• The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
• Links to related issues (if applicable)
• Tests for the changes have been added/updated (if applicable)
• Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
• For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

[RyanHolstien]

Thanks for the contribution! This is great, a couple of minor comments

[nmbryant]

@RyanHolstien Thanks for the review, I addressed your comments in the latest commit. Right now I have urnBasedPagination default to false, should I change the default value to true?

[nmbryant]

@RyanHolstien Updated the query a bit based on our results. With this change, we were able to complete RestoreIndices on ~66 million aspects in ~5 hours.

[RyanHolstien]

Confused on the usage of break here, was thinking this should be a failure condition where it does:

return new DefaultUpgradeStepResult(id(), UpgradeStepResult.Result.FAILED);

what was the idea around the break intended for?

[nmbryant]

Just wasn't aware that's how failure conditions should be handled, I'll get that updated

[RyanHolstien]

There are a lot of reformatting changes that got put into this PR that make it hard to see what has actually been changed

[nmbryant]

That's all from this PR: #9373
Do you know what was run to do the formatting for that PR so that I can run it on my changes?

[RyanHolstien]

I believe it's through Spotless which seems to be run through ./gradlew spotlessApply, might need to be run at a per module basis I haven't used it yet personally so haven't worked through the kinks myself yet. It might also work itself out when updating the branch which we can try.

[RyanHolstien]

Yeah merge did not fix it, it's showing that the lines got specifically changed from the new format to old format in this PR 😕

[RyanHolstien]

The build output gives the command: ./gradlew :metadata-service:services:spotlessApply should fix it

[nmbryant]

Thanks, that fixed it. I think everything should be good now

[sgomezvillamor]

I have suffered long upgrades because of RestoreIndices job in the past and this PR looks promising!
Is there any chance that this gets included in the next release @RyanHolstien?

[sixmen]

I found that it is much better if I added an index. (useful before the release)

create index version_urn_aspect on metadata_aspect_v2 (version, urn, aspect)

(a query form restoring is select urn, aspect, ... where version=0 order by urn, aspect limit n offset m)