Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Rebase strategy to pull/merge #933

Merged
merged 30 commits into from
Jan 10, 2025
Merged

Conversation

khanaffan
Copy link
Contributor

@khanaffan khanaffan commented Dec 6, 2024

Native changes

  1. Update SQLite from 3.47.0 to 3.47.2. It include a important fix needed by this PR. Its a bug I found that flag SQLITE_CHANGESETAPPLY_FKNOACTION does not work as we indented it to work and cause client to push inconsistent changes that cannot be applied by other briefcases.
  2. TxnManager::_OnCommit()
    • Separate DDL, ECSchema and Data changes and store them separately as individual txns.
  3. Older rebaser code removed. dgn_Rebase table is not used.
    • Removed invert parameter to TxnManager::ApplyChanges() its never used.
    • Removed TxnManager::SaveRebase
    • Removed TxnManager::LoadRebases
    • Removed TxnManager::DeleteRebases
    • Removed TxnManager::QueryLastRebaseId
    • Removed TxnManager::m_enableRebasers
    • Removed TxnManager::m_lastRebaseId
    • Removed ChangeTracker::m_hasEcSchemaChanges
    • Removed ChangeTracker::SetHasECSchemaChanges
    • Removed ChangeTracker::GetHasECSchemaChanges
    • Removed class ChangeStreamQueueProducer
    • Removed class ChangeStreamQueueConsumer
  4. ec_cache_* are tracked. This is required as reversing schema changeset does not work as ec_Class & ec_Table primary key is used as fk in ec_cache_* table.
  5. be_Local is not tracked anymore. This table should not have been tracked to start with. It ended up in changeset. We always delete everything from it when initializing briefcase capturing changes to it was a mistake.
  6. TxnManager::MergeDataChanges()
    • We do not propagate changes. MergeDataChanges() is called when applying incoming changeset. There is no need to propagate changes. its done when rebasing local txns.
  7. During ApplyChanges() we set fkNoAction only when not rebasing. This mean fast-forward reversing a local txn.
  8. TxnManager is not allowed any operation that changes txn when pull/merge is in progress.
  9. It is possible part of whole local txn disappear/deleted after pull merge. This happen because local changes is already part of incoming changes.
  10. If conflict during rebase is not resolved the Db will stay in state before reinstating conflicting changeset. User cannot call savechanges on it. They can only abandon changes.
  • Use resume() method to attempt to resolved may be in different session or even by different application. But once resolved rebase will be will completed.
  1. Native addon methods added
    • pullMergeInProgress(): boolean
    • pullMergeBegin(): void
    • pullMergeEnd(): void
    • pullMergeResume(): void
  2. Rebase api
    • Merge state is stored in be_Local. It is always read from be_Local.
    • TxnManager::PullMergeBegin()
      • It reverse all local txn.
      • It set current merge state of TxnManager to be in MergingRemoteChanges
    • TxnManager::PullMergeEnd()
      • It set current state of TxnManager to be RebasingLocalChanges
      • It generate ts event _onRebaseTxnBegin & _onRebaseTxnEnd when rebasing local txn.
      • It will invoke _onRebaseLocalTxnConflict if any conflict occure.
      • After rebase is successfull the rebased txn is used to propagate changes
      • After propagation the local txn is updated with rebased txn.
      • After all local reversed txn are rebased and reinstated The merge state of TxnManager is set to None
    • TxnManager::PullMergeResume()
      • Try to resume and rebase reversed txn and complete the rebase process.

itwinjs-core: iTwin/itwinjs-core#7458

@khanaffan khanaffan marked this pull request as ready for review December 6, 2024 18:34
@khanaffan
Copy link
Contributor Author

/azp run imodel-native

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@khanaffan
Copy link
Contributor Author

/azp run imodel-native

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pmconne
Copy link
Member

pmconne commented Dec 10, 2024

/azp run imodel-native

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pmconne
Copy link
Member

pmconne commented Dec 10, 2024

Please notify when builds are pssing.

@khanaffan
Copy link
Contributor Author

/azp run imodel-native

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@khanaffan
Copy link
Contributor Author

Please notify when builds are pssing.

its passing

@khanaffan khanaffan merged commit 5194324 into main Jan 10, 2025
19 checks passed
@khanaffan khanaffan deleted the affank/pull-merge-rebase-strategy branch January 10, 2025 16:13
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