-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
feature: make undo log table name configurable #1392
feature: make undo log table name configurable #1392
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1392 +/- ##
=============================================
- Coverage 45.43% 45.43% -0.01%
Complexity 1662 1662
=============================================
Files 345 345
Lines 12692 12699 +7
Branches 1608 1608
=============================================
+ Hits 5767 5770 +3
- Misses 6279 6282 +3
- Partials 646 647 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is there a need for this feature?
@@ -73,7 +75,8 @@ public int getValue() { | |||
} | |||
private static final Logger LOGGER = LoggerFactory.getLogger(UndoLogManagerOracle.class); | |||
|
|||
private static String UNDO_LOG_TABLE_NAME = "undo_log"; | |||
private static String UNDO_LOG_TABLE_NAME = ConfigurationFactory.getInstance() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Add final to all constants.
/** | ||
* The constant TRANSACTION_UNDO_LOG_DEFAULT_TABLE. | ||
*/ | ||
public static final String TRANSACTION_UNDO_LOG_DEFAULT_TABLE = "undo_log"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DEFAULT_TRANSACTION_UNDO_LOG_TABLE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just like STORE_DB_GLOBAL_DEFAULT_TABLE
or STORE_DB_BRANCH_DEFAULT_TABLE
.
DB table naming specifications vary. |
3e07575
to
dc2e169
Compare
But after the table created, there is no need for the business side to maintain it. |
@ggndnn please resolve conflicts. |
dc2e169
to
5fdb42b
Compare
5fdb42b
to
e12857c
Compare
Just give business side a chance to use different undo log table name other than |
@slievrly Conflicts resolved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okey to me
Ⅰ. Describe what this PR did
make undo log table name configurable
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews