From 609b5268b8997f44e186e9edbd464af3318b5905 Mon Sep 17 00:00:00 2001
From: Quanzheng Long <longquanzheng@users.noreply.github.com>
Date: Wed, 25 Sep 2019 11:20:31 -0700
Subject: [PATCH] Improve MySQL db query index usage (#2598)

---
 common/persistence/sql/sqlHistoryV2Manager.go      | 11 ++++++++++-
 common/persistence/sql/storage/mysql/eventsV2.go   |  4 ++--
 common/persistence/sql/storage/sqldb/interfaces.go |  4 +---
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/common/persistence/sql/sqlHistoryV2Manager.go b/common/persistence/sql/sqlHistoryV2Manager.go
index ca3dcf674bc..cfd3eecd849 100644
--- a/common/persistence/sql/sqlHistoryV2Manager.go
+++ b/common/persistence/sql/sqlHistoryV2Manager.go
@@ -192,7 +192,6 @@ func (m *sqlHistoryV2Manager) ReadHistoryBranch(
 		BranchID:  sqldb.MustParseUUID(request.BranchID),
 		MinNodeID: &minNodeID,
 		MaxNodeID: &maxNodeID,
-		MinTxnID:  &lastTxnID,
 		PageSize:  &request.PageSize,
 		ShardID:   request.ShardID,
 	}
@@ -220,6 +219,16 @@ func (m *sqlHistoryV2Manager) ReadHistoryBranch(
 			// event batches with larger node ID
 			//  -> batch with lower transaction ID is invalid (happens before)
 			//  -> batch with higher transaction ID is valid
+			if row.NodeID < lastNodeID {
+				return nil, &shared.InternalServiceError{
+					Message: fmt.Sprintf("corrupted data, nodeID cannot decrease"),
+				}
+			} else if row.NodeID > lastNodeID {
+				// update lastNodeID so that our pagination can make progress in the corner case that
+				// the page are all rows with smaller txnID
+				// because next page we always have minNodeID = lastNodeID+1
+				lastNodeID = row.NodeID
+			}
 			continue
 		}
 
diff --git a/common/persistence/sql/storage/mysql/eventsV2.go b/common/persistence/sql/storage/mysql/eventsV2.go
index 465cbe3bd3e..1b33f775f18 100644
--- a/common/persistence/sql/storage/mysql/eventsV2.go
+++ b/common/persistence/sql/storage/mysql/eventsV2.go
@@ -33,7 +33,7 @@ const (
 		`VALUES (:shard_id, :tree_id, :branch_id, :node_id, :txn_id, :data, :data_encoding) `
 
 	getHistoryNodesQry = `SELECT node_id, txn_id, data, data_encoding FROM history_node ` +
-		`WHERE shard_id = ? AND tree_id = ? AND branch_id = ? AND node_id >= ? and node_id < ? and txn_id < ? ORDER BY shard_id, tree_id, branch_id, node_id, txn_id LIMIT ? `
+		`WHERE shard_id = ? AND tree_id = ? AND branch_id = ? AND node_id >= ? and node_id < ? ORDER BY shard_id, tree_id, branch_id, node_id, txn_id LIMIT ? `
 
 	deleteHistoryNodesQry = `DELETE FROM history_node WHERE shard_id = ? AND tree_id = ? AND branch_id = ? AND node_id >= ? `
 
@@ -62,7 +62,7 @@ func (mdb *DB) InsertIntoHistoryNode(row *sqldb.HistoryNodeRow) (sql.Result, err
 func (mdb *DB) SelectFromHistoryNode(filter *sqldb.HistoryNodeFilter) ([]sqldb.HistoryNodeRow, error) {
 	var rows []sqldb.HistoryNodeRow
 	err := mdb.conn.Select(&rows, getHistoryNodesQry,
-		filter.ShardID, filter.TreeID, filter.BranchID, *filter.MinNodeID, *filter.MaxNodeID, -*filter.MinTxnID, *filter.PageSize)
+		filter.ShardID, filter.TreeID, filter.BranchID, *filter.MinNodeID, *filter.MaxNodeID, *filter.PageSize)
 	// NOTE: since we let txn_id multiple by -1 when inserting, we have to revert it back here
 	for _, row := range rows {
 		*row.TxnID *= -1
diff --git a/common/persistence/sql/storage/sqldb/interfaces.go b/common/persistence/sql/storage/sqldb/interfaces.go
index 9450e16eff5..bc270577160 100644
--- a/common/persistence/sql/storage/sqldb/interfaces.go
+++ b/common/persistence/sql/storage/sqldb/interfaces.go
@@ -278,9 +278,7 @@ type (
 		MinNodeID *int64
 		// Exclusive
 		MaxNodeID *int64
-		// Exclusive
-		MinTxnID *int64
-		PageSize *int
+		PageSize  *int
 	}
 
 	// HistoryTreeRow represents a row in history_tree table