-
Notifications
You must be signed in to change notification settings - Fork 184
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
Retry CloseAuctions procedure in AuctionMark on rollback #354
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -447,8 +447,24 @@ protected boolean executeCloseAuctions(Connection conn, CloseAuctions proc) thro | |
Timestamp startTime = profile.getLastCloseAuctionsTime(); | ||
Timestamp endTime = profile.updateAndGetLastCloseAuctionsTime(); | ||
|
||
List<Object[]> results = proc.run(conn, benchmarkTimes, startTime, endTime); | ||
|
||
List<Object[]> results = null; | ||
boolean done = false; | ||
// Retry here the close auction procedure if it fails due to a transactional conflict, since | ||
// the original submitted procedured has a different type. | ||
while (!done) { | ||
try { | ||
results = proc.run(conn, benchmarkTimes, startTime, endTime); | ||
done = true; | ||
} | ||
catch (SQLException e) { | ||
if (e.getSQLState().startsWith("40")) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is "40"? Is this DBMS specific? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SQL states starting with 40 are related to transactional conflicts, such as "serialization failure" and "integrity constraint violation". As far as I know, it is standard across SQL systems. |
||
conn.rollback(); | ||
} | ||
Comment on lines
+454
to
+462
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this get stuck in an infinite loop? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so, because sooner or later the transaction will be able to complete. Any other type of error other than transactional conflicts will raise an exception to the worker. |
||
else { | ||
throw e; | ||
} | ||
} | ||
} | ||
|
||
for (Object[] row : results) { | ||
ItemId itemId = this.processItemRecord(row); | ||
|
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.
I actually don't think we want to do this because we can't keep track of the # of times that a txn is submitted. So this can cause BenchBase to exceed the defined submission rate. Also, by retrying inside of the benchmark worker we can't keep track of the # failed txns.
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.
I also agree that this is not the best approach. I also thought about adding an update method to the "TransactionType" class, so we can update the original "txnType" when we switch it in the "executeWork" function, making the worker deal with retries. This also would mean that the CloseAuctions procedure would be correctly logged in the results, since right now, as far as the worker knows, no CloseAuctions procedure is executed.