From 49a614eae8bd05725fbff167e31347acd91f26b5 Mon Sep 17 00:00:00 2001 From: "alex.sharov" Date: Wed, 4 Dec 2019 10:55:08 +0700 Subject: [PATCH] Bug fixes: - rollback must catch local variable, if you write value to lastError variable - then you can't do "return" - because it closing connection and client can't send CmdLastError - somewhere error values was ommited in loging --- cmd/state/stateless/state.go | 20 ++++++++++++-------- ethdb/remote/bolt_remote.go | 27 +++++++++++++++------------ ethdb/remote/bolt_remote_test.go | 26 +++++++++++++------------- 3 files changed, 40 insertions(+), 33 deletions(-) diff --git a/cmd/state/stateless/state.go b/cmd/state/stateless/state.go index 56529c358..d69e62cb0 100644 --- a/cmd/state/stateless/state.go +++ b/cmd/state/stateless/state.go @@ -136,7 +136,7 @@ func (r *Reporter) StateGrowth1(chaindata string) { creationsByBlock := make(map[uint64]int) var addrHash common.Hash // Go through the history of account first - if err := r.db.View(func(tx *remote.Tx) error { + err = r.db.View(func(tx *remote.Tx) error { b := tx.Bucket(dbutils.AccountsHistoryBucket) if b == nil { return nil @@ -163,14 +163,15 @@ func (r *Reporter) StateGrowth1(chaindata string) { } } return nil - }); err != nil { + }) + if err != nil { r.db.Close() r.db = nil check(err) } // Go through the current state - if err := r.db.View(func(tx *remote.Tx) error { + err = r.db.View(func(tx *remote.Tx) error { pre := tx.Bucket(dbutils.PreimagePrefix) if pre == nil { return nil @@ -190,7 +191,8 @@ func (r *Reporter) StateGrowth1(chaindata string) { } } return nil - }); err != nil { + }) + if err != nil { r.db.Close() r.db = nil check(err) @@ -241,7 +243,7 @@ func (r *Reporter) StateGrowth2(chaindata string) { var addrHash common.Hash var hash common.Hash // Go through the history of account first - if err = r.db.View(func(tx *remote.Tx) error { + err = r.db.View(func(tx *remote.Tx) error { b := tx.Bucket(dbutils.StorageHistoryBucket) if b == nil { return nil @@ -284,14 +286,15 @@ func (r *Reporter) StateGrowth2(chaindata string) { } } return nil - }); err != nil { + }) + if err != nil { r.db.Close() r.db = nil check(err) } // Go through the current state - if err = r.db.View(func(tx *remote.Tx) error { + err = r.db.View(func(tx *remote.Tx) error { b := tx.Bucket(dbutils.StorageBucket) if b == nil { return nil @@ -312,7 +315,8 @@ func (r *Reporter) StateGrowth2(chaindata string) { } } return nil - }); err != nil { + }) + if err != nil { r.db.Close() r.db = nil check(err) diff --git a/ethdb/remote/bolt_remote.go b/ethdb/remote/bolt_remote.go index 75b233e49..bf313a652 100644 --- a/ethdb/remote/bolt_remote.go +++ b/ethdb/remote/bolt_remote.go @@ -180,7 +180,9 @@ func Server(db *bolt.DB, in io.Reader, out io.Writer, closer io.Closer) error { // We do Rollback and never Commit, because the remote transactions are always read-only, and must never change // anything // nolint:errcheck - defer tx.Rollback() + defer func(tx *bolt.Tx) { + tx.Rollback() + }(tx) lastHandle++ txHandle = lastHandle transactions[txHandle] = tx @@ -192,13 +194,14 @@ func Server(db *bolt.DB, in io.Reader, out io.Writer, closer io.Closer) error { case CmdEndTx: var txHandle uint64 if err := decoder.Decode(&txHandle); err != nil { - log.Error("could not decode txHandle for CmdEndTx") + log.Error("could not decode txHandle for CmdEndTx, %v", err) return err } tx, ok := transactions[txHandle] if !ok { lastError = fmt.Errorf("transaction not found") - return nil + continue + //return nil } // Remove all the buckets @@ -225,7 +228,7 @@ func Server(db *bolt.DB, in io.Reader, out io.Writer, closer io.Closer) error { // Read the txHandle var txHandle uint64 if err := decoder.Decode(&txHandle); err != nil { - log.Error("could not decode txHandle for CmdBucket") + log.Error("could not decode txHandle for CmdBucket, %v", err) return err } // Read the name of the bucket @@ -263,12 +266,12 @@ func Server(db *bolt.DB, in io.Reader, out io.Writer, closer io.Closer) error { case CmdGet: var bucketHandle uint64 if err := decoder.Decode(&bucketHandle); err != nil { - log.Error("could not decode bucketHandle for CmdGet") + log.Error("could not decode bucketHandle for CmdGet, %v", err) return err } var key []byte if err := decoder.Decode(&key); err != nil { - log.Error("could not decode key for CmdGet") + log.Error("could not decode key for CmdGet, %v", err) return err } var value []byte @@ -339,19 +342,19 @@ func Server(db *bolt.DB, in io.Reader, out io.Writer, closer io.Closer) error { var err error if err := decoder.Decode(&cursorHandle); err != nil { - log.Error("could not decode cursorHandle for CmdCursorNext") + log.Error("could not decode cursorHandle for CmdCursorNext, %v", err) return err } var numberOfKeys uint64 if err := decoder.Decode(&numberOfKeys); err != nil { - log.Error("could not decode numberOfKeys for CmdCursorNext") + log.Error("could not decode numberOfKeys for CmdCursorNext, %v", err) } var key, value []byte cursor, ok := cursors[cursorHandle] if !ok { lastError = fmt.Errorf("cursor not found") - return nil + continue } for numberOfKeys > 0 { @@ -376,18 +379,18 @@ func Server(db *bolt.DB, in io.Reader, out io.Writer, closer io.Closer) error { case CmdCursorFirst: var cursorHandle uint64 if err := decoder.Decode(&cursorHandle); err != nil { - log.Error("could not decode cursorHandle for CmdCursorFirst") + log.Error("could not decode cursorHandle for CmdCursorFirst, %v", err) return err } var numberOfKeys uint64 if err := decoder.Decode(&numberOfKeys); err != nil { - log.Error("could not decode numberOfKeys for CmdCursorFirst") + log.Error("could not decode numberOfKeys for CmdCursorFirst, %v", err) } var key, value []byte cursor, ok := cursors[cursorHandle] if !ok { lastError = fmt.Errorf("cursor not found") - return nil + continue } key, value = cursor.First() diff --git a/ethdb/remote/bolt_remote_test.go b/ethdb/remote/bolt_remote_test.go index 78912bf71..f8381740a 100644 --- a/ethdb/remote/bolt_remote_test.go +++ b/ethdb/remote/bolt_remote_test.go @@ -100,7 +100,7 @@ func TestCmdBeginEndLastError(t *testing.T) { if err = encoder.Encode(&txHandle); err != nil { t.Errorf("Could not encode txHandle: %v", err) } - // CmdLastError to retrive the error related to the CmdEndTx with the wrong handle + // CmdLastError to retrieve the error related to the CmdEndTx with the wrong handle c = CmdLastError if err = encoder.Encode(&c); err != nil { t.Errorf("Could not encode CmdLastError: %v", err) @@ -126,17 +126,17 @@ func TestCmdBeginEndLastError(t *testing.T) { } // And then we interpret the results if err = decoder.Decode(&txHandle); err != nil { - t.Errorf("Could not decode response from CmdBeginTx") + t.Errorf("Could not decode response from CmdBeginTx, %v", err) } var lastErrorStr string if err = decoder.Decode(&lastErrorStr); err != nil { - t.Errorf("Could not decode response from CmdLastError") + t.Errorf("Could not decode response from CmdLastError, %v", err) } if lastErrorStr != "transaction not found" { t.Errorf("Wrong error message from CmdLastError: %s", lastErrorStr) } if err = decoder.Decode(&lastErrorStr); err != nil { - t.Errorf("Could not decode response from CmdLastError") + t.Errorf("Could not decode response from CmdLastError, %v", err) } if lastErrorStr != "" { t.Errorf("Wrong error message from CmdLastError: %s", lastErrorStr) @@ -188,14 +188,14 @@ func TestCmdBucket(t *testing.T) { } // And then we interpret the results if err = decoder.Decode(&txHandle); err != nil { - t.Errorf("Could not decode response from CmdBegin") + t.Errorf("Could not decode response from CmdBegin, %v", err) } if txHandle != 1 { t.Errorf("Unexpected txHandle: %d", txHandle) } var bucketHandle uint64 if err = decoder.Decode(&bucketHandle); err != nil { - t.Errorf("Could not decode response from CmdBucket") + t.Errorf("Could not decode response from CmdBucket, %v", err) } if bucketHandle != 2 { t.Errorf("Unexpected bucketHandle: %d", bucketHandle) @@ -282,14 +282,14 @@ func TestCmdGet(t *testing.T) { // And then we interpret the results // Results of CmdBeginTx if err = decoder.Decode(&txHandle); err != nil { - t.Errorf("Could not decode response from CmdBegin") + t.Errorf("Could not decode response from CmdBegin, %v", err) } if txHandle != 1 { t.Errorf("Unexpected txHandle: %d", txHandle) } // Results of CmdBucket if err = decoder.Decode(&bucketHandle); err != nil { - t.Errorf("Could not decode response from CmdBucket") + t.Errorf("Could not decode response from CmdBucket, %v", err) } if bucketHandle != 2 { t.Errorf("Unexpected bucketHandle: %d", bucketHandle) @@ -386,14 +386,14 @@ func TestCmdSeek(t *testing.T) { // And then we interpret the results // Results of CmdBeginTx if err = decoder.Decode(&txHandle); err != nil { - t.Errorf("Could not decode response from CmdBegin") + t.Errorf("Could not decode response from CmdBegin, %v", err) } if txHandle != 1 { t.Errorf("Unexpected txHandle: %d", txHandle) } // Results of CmdBucket if err = decoder.Decode(&bucketHandle); err != nil { - t.Errorf("Could not decode response from CmdBucket") + t.Errorf("Could not decode response from CmdBucket, %v", err) } if bucketHandle != 2 { t.Errorf("Unexpected bucketHandle: %d", bucketHandle) @@ -497,7 +497,7 @@ func TestCmdNext(t *testing.T) { } var numberOfKeys uint64 = 3 // Trying to get 3 keys, but will get 1 + nil if err = encoder.Encode(&numberOfKeys); err != nil { - t.Errorf("Could not encode numberOfKeys for CmdNex: %v", err) + t.Errorf("Could not encode numberOfKeys for CmdCursorNext: %v", err) } // By now we constructed all input requests, now we call the // Server to process them all @@ -507,14 +507,14 @@ func TestCmdNext(t *testing.T) { // And then we interpret the results // Results of CmdBeginTx if err = decoder.Decode(&txHandle); err != nil { - t.Errorf("Could not decode response from CmdBegin") + t.Errorf("Could not decode response from CmdBegin, %v", err) } if txHandle != 1 { t.Errorf("Unexpected txHandle: %d", txHandle) } // Results of CmdBucket if err = decoder.Decode(&bucketHandle); err != nil { - t.Errorf("Could not decode response from CmdBucket") + t.Errorf("Could not decode response from CmdBucket, %v", err) } if bucketHandle != 2 { t.Errorf("Unexpected bucketHandle: %d", bucketHandle)