Skip to content

Globally Unique FATE Transaction Ids - Part 2#4228

Merged
keith-turner merged 5 commits into
apache:elasticityfrom
kevinrr888:elasticity-feature-4044-2
Feb 5, 2024
Merged

Globally Unique FATE Transaction Ids - Part 2#4228
keith-turner merged 5 commits into
apache:elasticityfrom
kevinrr888:elasticity-feature-4044-2

Conversation

@kevinrr888
Copy link
Copy Markdown
Member

This is a follow up PR to #4191 for #4044. This PR addresses the task "Change Repo to use FateId. Resolve issues stemming from these changes." This PR includes the following changes:

  • ReadOnlyRepo updated to use FateId
    • Repo updated to use FateId (implements ReadOnlyRepo)
      • FateIT updated to use FateId (implements Repo)
      • TraceRepo updated to use FateId (implements Repo)
      • ManagerRepo updated to use FateId (implements Repo)
        • All 62 implementations of ManagerRepo updated to use FateId
  • Utils updated to use FateId (used by many of the ManagerRepo implementations)
  • Fate updated to pass the FateId to Repo method calls
  • Tests PrepBulkImportTest and ShutdownTServerTest updated
  • Minor change to FateId

(72 file changes)
There are many places in the code marked "ELASTICITY_TODO DEFERRED - ISSUE 4044" which will be addressed in later PR(s)

@kevinrr888 kevinrr888 changed the title Change Repo to use FateId Globally Unique FATE Transaction Ids - Part 2 Feb 5, 2024
@kevinrr888 kevinrr888 marked this pull request as ready for review February 5, 2024 18:40
try (LoadMappingIterator lmi = createLoadMappingIter(loadRanges)) {
var extent =
PrepBulkImport.validateLoadMapping("1", lmi, tabletIterFactory, maxTablets, 10001);
var extent = PrepBulkImport.validateLoadMapping("1", lmi, tabletIterFactory, maxTablets);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was 10001 just an unused method parameter? I thought I saw another place in the code where it was remove in the review.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, validateLoadMapping() never used the transaction id

@keith-turner keith-turner merged commit b1cf91c into apache:elasticity Feb 5, 2024
@keith-turner
Copy link
Copy Markdown
Contributor

@kevinrr888 I added a few comments and then committed this. Wanted to get it in since it was such a big change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants