cshannon commented on code in PR #4204:
URL: https://github.com/apache/accumulo/pull/4204#discussion_r1471060651
##########
core/src/main/java/org/apache/accumulo/core/fate/ZooStore.java:
##########
@@ -91,6 +94,27 @@ public long create() {
}
}
+ @Override
+ protected void create(long tid, byte[] key) {
+ // TODO: Should we somehow make this Atomic or clean up on failure to make
sure
+ // that either both of these writes happen or none happen?
+ try {
+ zk.putPersistentData(getTXPath(tid), TStatus.NEW.name().getBytes(UTF_8),
+ NodeExistsPolicy.FAIL);
+ // The key was already used to generate the tid but we still need to
store it
+ // separate to check later for collision detection
+ zk.putPersistentData(getTXPath(tid) + "/" + TxInfo.TX_KEY,
serializeTxInfo(key),
Review Comment:
Thanks for pointing out how you handled it in
https://github.com/apache/accumulo/pull/3964, I will take a look at making the
update now in this PR but if the changes are too large I can do it in a
separate PR and keep it two separate operations for now.
I am hoping the change wouldn't be too bad for now if I follow the same
pattern as #3964 and as you noted there how we actually serialize could change
later (json or something else).
For a follow on issue there has been a lot of talk about getting rid of the
Java serialization in FATE anyways and using something else (including in the
main branch) so maybe the serialization format could be looked at then.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
