[GitHub] [ignite-3] vldpyatkov commented on a diff in pull request #1515: IGNITE-18522

[GitBox <gi...@apache.org>]

vldpyatkov commented on code in PR #1515:
URL: https://github.com/apache/ignite-3/pull/1515#discussion_r1069121431


##########
modules/transactions/README.md:
##########
@@ -12,7 +12,7 @@ like parallel commits from <sup id="a1">[1](#f1)</sup>
 
 # Transaction protocol design (first iteration)
 At a high level, we utilize 2 phase locking (2PL) for concurrency control, 2 phase commit (2PC) as an atomic commitment 

Review Comment:
   I don't think we can to define our protocol as a kind of 2PC.
   Also, nowhere I see that our transactions are related to the class of pessimistic one.



##########
modules/transactions/README.md:
##########
@@ -63,13 +63,16 @@ TX coordination can be done from any grid node. Coordinators can be dedicated no
 Coordinators are responsible for id assignment, tx mapping and failover handling if some nodes from tx topology have failed. 
 Knows full tx topology just before committing.
 
+# Transaction id and timestamp
+Transaction is identified by UUID and incorporates timestamp.

Review Comment:
   Maybe we can more clearly describe how UUID includes a timestamp itself.
   Point to the code to UUID generation.



##########
modules/transactions/README.md:
##########
@@ -63,13 +63,16 @@ TX coordination can be done from any grid node. Coordinators can be dedicated no
 Coordinators are responsible for id assignment, tx mapping and failover handling if some nodes from tx topology have failed. 
 Knows full tx topology just before committing.
 
+# Transaction id and timestamp
+Transaction is identified by UUID and incorporates timestamp.
+
 # Deadlock prevention
-Deadlock prevention in WAIT_DIE mode (described in details in <sup id="a2">[2](#f2)</sup>)- uses tx priorities to decide which tx should be restarted.
+Deadlock detection is not an option due to huge computation resources requirement and no real-time guaranties.

Review Comment:
   This phrase does not look like as technical one.
   I think, required to reformulate this or remove at all.



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org