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

[LEDGER] remove TxMgr interface #1095

Closed
wants to merge 2 commits into from

Conversation

cendhu
Copy link
Contributor

@cendhu cendhu commented Apr 15, 2020

Type of change

  • Improvement (improvement to code)

Description

We have a TxMgr interface as the initial plan was to implement two types of transaction manager:

  1. lock-based txmgr
  2. snapshot isolation based txmgr

In the last three years and in the many coming years, we may not implement another TxMgr. Hence, we remove the interface.

Additional details

As we decided to keep a single TxMgr implementation, we can avoid additional folder depth and reduce a package by moving lockbasedtxmgr to txmgr.

TxStatInfo defined in txmgr is imported in both lockbasedtxmgr and validator. If we move the lockbasedtxmgr to txmgr, we will get an import cycle. As TxStatInfo is used by kvledger, metrics, lockbasedtxmgr, and validator, we need to find a suitable place to keep these structs. Only if we can do that, we can move lockbasedtxmgr to txmgr -- FAB-17757

Merge this only after merging #1094

Related issues

FAB-17756

cendhu added 2 commits April 15, 2020 16:15
In the validator, we already create a mock for TxMgr interface
but use the mock created at txmgr package. Hence, we make the
validator to use the mock that present locally.

Further, the validator does not need to create mock for all APIs
in the TxMgr interface as it only uses NewTxSimulator(). Hence,
we define a local interface.

Signed-off-by: senthil <cendhu@gmail.com>
We have a TxMgr interface as the initial plan was
to implement two types of transaction manager:
  (1) lock based txmgr
  (2) snapshot isolation based txmgr
In the last three years and in the many coming years, we may
not implement another txmgr. Hence, we can remove the interface.

Signed-off-by: senthil <cendhu@gmail.com>
@cendhu cendhu requested a review from a team as a code owner April 15, 2020 11:24
@cendhu cendhu marked this pull request as draft April 15, 2020 16:27
@cendhu
Copy link
Contributor Author

cendhu commented Apr 15, 2020

We will reopen this once we finish refactoring the mvcc validator.

@cendhu cendhu closed this Apr 15, 2020
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.

1 participant