HDDS-4660. Update OMTransactionInfo to TransactionInfo with functions added #1833
Conversation
|
also R: @GlenGeng @runzhiwang |
|
@hanishakoneru , can you please check this ?? |
There was a problem hiding this comment.
Thanks @amaliujia for working on this.
Patch LGTM overall.
Did you test for compatibility? It does not look like there is incompatibility but better to be sure.
| this.term = currentTerm; | ||
| this.transactionIndex = transactionIndex; | ||
| } | ||
|
|
||
| public boolean isInitialized() { | ||
| return transactionIndex == -1 && term == 0; |
There was a problem hiding this comment.
This function is not being called anywhere.
Also, if initialized, then txIndex should not be equal to -1, right?
| return this.getTransactionIndex() <= info.getTransactionIndex() ? -1 : 1; | ||
| } else { | ||
| return this.getTerm() < info.getTerm() ? -1 : 1; | ||
| } |
There was a problem hiding this comment.
I am not sure if I am missing something but couldn't find the usage for this function.
And shouldn't compareTo() return 0 if the TransactionInfo's are same?
There was a problem hiding this comment.
@amaliujia , can we move TransactionInfo class to hdds-common probbaly (or another suitable package), so that reusability in both SCM and OM becomes easier.
What changes were proposed in this pull request?
Requirements:
By doing so, we can use the TransactionInfo in HDDS-2823 and remove duplicate code
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4660
How was this patch tested?
UT