storage/mysql: always write leaves to Unsequenced in hash order (#408)

If distinct multi-row write operations for the Unsequenced table (i.e.
SequenceBatch, QueueLeaves) use arbitrary ordering, the chances of
DB database deadlock are increased.

AIUI each write will lock a range of index values to preserve primary
key uniqueness, roughly [prev-existing-hash, new-hash].

So one multiple row update might lock (say) BCD then LMN then WXYZ.
Meanwhile, a different transaction might try to lock UVW then DEF.

This gives a chance of deadlock:
 1) gets BCD and LMN
 2) gets UVW
 1) tries to get WXYZ and needs to wait for 2)
 2) tries to get DEF and needs to wait for 1).

storage/mysql/log_storage.go

@@ -15,13 +15,15 @@
 package mysql
 
 import (
+	"bytes"
 	"context"
 	"crypto/rand"
 	"crypto/sha256"
 	"database/sql"
 	"encoding/binary"
 	"errors"
 	"fmt"
+	"sort"
 	"strings"
 	"time"
 
@@ -320,7 +322,12 @@ func (t *logTreeTX) QueueLeaves(leaves []*trillian.LogLeaf, queueTimestamp time.
 		insertSQL = insertUnsequencedLeafSQLNoDuplicates
 	}
 
-	for i, leaf := range leaves {
+	// Insert in order of the hash values in the leaves.
+	orderedLeaves := make([]*trillian.LogLeaf, len(leaves))
+	copy(orderedLeaves, leaves)
+	sort.Sort(byLeafIdentityHash(orderedLeaves))
+
+	for i, leaf := range orderedLeaves {
 		// Create the unsequenced leaf data entry. We don't use INSERT IGNORE because this
 		// can suppress errors unrelated to key collisions. We don't use REPLACE because
 		// if there's ever a hash collision it will do the wrong thing and it also
@@ -510,7 +517,12 @@ func (t *logTreeTX) UpdateSequencedLeaves(leaves []*trillian.LogLeaf) error {
 	return nil
 }
 
+// removeSequencedLeaves removes the passed in leaves slice (which may be
+// modified as part of the operation).
 func (t *logTreeTX) removeSequencedLeaves(leaves []*trillian.LogLeaf) error {
+	// Delete in order of the hash values in the leaves.
+	sort.Sort(byLeafIdentityHash(leaves))
+
 	tmpl, err := t.ls.getDeleteUnsequencedStmt(len(leaves))
 	if err != nil {
 		glog.Warningf("Failed to get delete statement for sequenced work: %s", err)
@@ -583,3 +595,15 @@ func (t *logTreeTX) GetActiveLogIDs() ([]int64, error) {
 func (t *logTreeTX) GetActiveLogIDsWithPendingWork() ([]int64, error) {
 	return getActiveLogIDsWithPendingWork(t.tx)
 }
+
+type byLeafIdentityHash []*trillian.LogLeaf
+
+func (l byLeafIdentityHash) Len() int {
+	return len(l)
+}
+func (l byLeafIdentityHash) Swap(i, j int) {
+	l[i], l[j] = l[j], l[i]
+}
+func (l byLeafIdentityHash) Less(i, j int) bool {
+	return bytes.Compare(l[i].LeafIdentityHash, l[j].LeafIdentityHash) == -1
+}

storage/mysql/log_storage_test.go

@@ -21,6 +21,7 @@ import (
 	"database/sql"
 	"fmt"
 	"reflect"
+	"sort"
 	"strings"
 	"testing"
 	"time"
@@ -960,6 +961,30 @@ func TestGetSequencedLeafCount(t *testing.T) {
 	commit(tx, t)
 }
 
+func TestSortByLeafIdentityHash(t *testing.T) {
+	l := make([]*trillian.LogLeaf, 30)
+	for i := range l {
+		hash := sha256.Sum256([]byte{byte(i)})
+		leaf := trillian.LogLeaf{
+			LeafIdentityHash: hash[:],
+			LeafValue:        []byte(fmt.Sprintf("Value %d", i)),
+			ExtraData:        []byte(fmt.Sprintf("Extra %d", i)),
+			LeafIndex:        int64(i),
+		}
+		l[i] = &leaf
+	}
+	sort.Sort(byLeafIdentityHash(l))
+	for i := range l {
+		if i == 0 {
+			continue
+		}
+		if bytes.Compare(l[i-1].LeafIdentityHash, l[i].LeafIdentityHash) != -1 {
+			t.Errorf("sorted leaves not in order, [%d] = %x, [%d] = %x", i-1, l[i-1].LeafIdentityHash, i, l[i].LeafIdentityHash)
+		}
+	}
+
+}
+
 func ensureAllLeavesDistinct(leaves []*trillian.LogLeaf, t *testing.T) {
 	// All the leaf value hashes should be distinct because the leaves were created with distinct
 	// leaf data. If only we had maps with slices as keys or sets or pretty much any kind of usable