Frameworkbenchmarks: [Go/fasthttp] Benchmark requirement violation(s)

[mwpastore]

The updateHandler() method in server-postgresql.go and server-mysql.go uses a single transaction to perform multiple writes. Per the benchmark requirements, "a transaction should only encapsulate a single iteration, composed of a single read and single write."

[nbrady-techempower]

@mwpastore It appears to me that they're in for loops, making a single transaction. Is this not what's going on in code?

[mwpastore]

@nbrady-techempower My reading is that the transaction is started before the for-loops, and committed afterwards, so the transaction is actually writing multiple records.

[nbrady-techempower]

https://github.com/TechEmpower/FrameworkBenchmarks/blob/master/frameworks/Go/fasthttp/src/server-postgresql/server.go#L122 I believe this is the write transaction?

After the for loops is just the JSON response.

[mwpastore]

@nbrady-techempower That's executing a query within the transaction, but the transaction itself isn't committed until this statement. The updates don't take effect until the transaction is committed, so in effect the transaction encapsulates multiple writes.

[nbrady-techempower]

@mwpastore Aha! You are absolutely right. I didn't see that line and assumed .Exec was completing the transaction.

ping @valyala did you have any comments on this?

Otherwise, this definitely needs to be rectified per these rules

[valyala]

fasthttp issues a separate request per each selected row via fetchRandomWorld. Then it opens a transaction at this line and writes updated rows in a single batch, committing the transaction at this line. IMHO, this conforms to the following rule:

1. Use of batch updates is acceptable but not required. To be clear: batches are not permissible for selecting/reading the rows, but batches are acceptable for writing the updates.

The sole purpose of the transaction here is for writing updates in a single batch. Rows are fetched before (outside) the transaction. The update transaction may be substituted by a single UPDATE statement for all the rows, but this will complicate the code and move it away from the code usually used in the production.

[mwpastore]

@valyala Not to make too big a stink about this, but I disagree with your assessment. The next rule states:

1. Use of transactions is acceptable but not required. If transactions are used, a transaction should only encapsulate a single iteration, composed of a single read and single write. Transactions should not be used to consolidate multiple iterations into a single operation.

The way this benchmark is constructed results in the following SQL commands being issued:

BEGIN
UPDATE
UPDATE
UPDATE
..
UPDATE
COMMIT

Multiple writes are encapsulated into a single transaction, which violates this rule. Conformant commands would look more like:

BEGIN
UPDATE
COMMIT
BEGIN
UPDATE
COMMIT
..
BEGIN
UPDATE
COMMIT

Or, encapsulating the read as well:

BEGIN
SELECT FOR UPDATE
UPDATE
COMMIT
BEGIN
SELECT FOR UPDATE
UPDATE
COMMIT
..
BEGIN
SELECT FOR UPDATE
UPDATE
COMMIT

All that being said, I think the rules are confusing, or at least confusingly-worded, and I've raised my "concerns" about this elsewhere.

[nbrady-techempower]

@msmith-techempower any input on this?

[bhauer]

@mwpastore I recently changed the wording of clause 10 to remove the constraints on transactions. This makes it more permissive, which I think is the right thing considering we've already conceded that use of batch updates is acceptable.

Let me know if you think that helps clarify and address the confusing wording or if it just makes things worse. :)