-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
support to delete undolog in back task #1235
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1235 +/- ##
=============================================
- Coverage 48.6% 48.32% -0.29%
- Complexity 1645 1653 +8
=============================================
Files 331 333 +2
Lines 11535 11669 +134
Branches 1430 1448 +18
=============================================
+ Hits 5607 5639 +32
- Misses 5306 5401 +95
- Partials 622 629 +7
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.
LGTM.
@Override | ||
public AbstractTransactionResponse handle(RpcContext rpcContext) { | ||
handler.handle(this); | ||
return null; |
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 return null ?
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 return null ?
async,we don't need reply
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.
TODO: UndoLogDeleteRequest needs to support PB later
byteBuffer.putShort((short)0); | ||
} | ||
//3.save days | ||
byteBuffer.putInt(saveDays); |
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.
probably short enough
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
} | ||
Map<String,Channel> channels = new HashMap(RM_CHANNELS.size()); | ||
for (String resourceId : RM_CHANNELS.keySet()) { | ||
Channel channel = tryOtherApp(RM_CHANNELS.get(resourceId), null); |
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 tryOtherApp? tryOtherApp just get from RM_CHANNELS, loop is also getting from RM_CHANNELS and your map can only save one channel for a resource
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.
the method i have modified,when the param applitioncation is null,will find one avtive channel from all application.
/** | ||
* The Timeout retry delay. | ||
*/ | ||
protected int transactionUndologDeleteDelay = CONFIG.getInt(ConfigurationKeys.TRANSACTION_UNDO_LOG_DELETE_DELAY, 24 * 60); |
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.
best to indicate the time unit
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.
same with committingRetryDelay,asynCommittingRetryDelay,timeoutRetryDelay, default is SECONDS
…_del_backup # Conflicts: # core/src/main/java/io/seata/core/constants/ConfigurationKeys.java # core/src/main/java/io/seata/core/protocol/AbstractMessage.java
…scar into feature_del_backup
…_del_backup # Conflicts: # core/src/main/java/io/seata/core/rpc/ServerMessageSender.java # core/src/main/java/io/seata/core/rpc/netty/RmMessageListener.java # core/src/main/java/io/seata/core/rpc/netty/RpcServer.java
…scar into feature_del_backup
…_del_backup # Conflicts: # rm-datasource/src/main/java/io/seata/rm/datasource/undo/UndoLogManager.java
…scar into feature_del_backup
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.
Please add the configuration item to the configuration file.
…scar into feature_del_backup
done |
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
Ⅰ. Describe what this PR did
support to delete undolog in back task
Ⅱ. Does this pull request fix one issue?
a feature,some case the undolog will not delete ,the pr can avoid the undo_log table so big
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews