eth/downloader: flush state data before exit #16280

rjl493456442 · 2018-03-08T02:30:41Z

All trie nodes committed to TrieSync.membatch is completed. So we should flush the membatch data to underlying database when we receive a downloader exit signal to prevent data loss.

karalabe · 2018-03-16T12:02:54Z

trie/sync.go

@@ -212,7 +212,7 @@ func (s *TrieSync) Process(results []SyncResult) (bool, int, error) {
 }

 // Commit flushes the data stored in the internal membatch out to persistent
-// storage, returning th enumber of items written and any occurred error.
+// storage, returning th number of items written and any occurred error.


"the number"

karalabe · 2018-03-16T12:03:37Z

eth/downloader/statesync.go

@@ -323,7 +326,10 @@ func (s *stateSync) commit(force bool) error {
 	}
 	start := time.Now()
 	b := s.d.stateDB.NewBatch()
-	s.sched.Commit(b)
+	// Ignore empty write.


I don't think you need this comment here, it's fairly obvious.

karalabe · 2018-03-16T12:05:33Z

eth/downloader/statesync.go

 	// Listen for new peer events to assign tasks to them
 	newPeer := make(chan *peerConnection, 1024)
 	peerSub := s.d.peers.SubscribeNewPeers(newPeer)
 	defer peerSub.Unsubscribe()
+	defer func() {
+		err = s.commit(true)
+	}()


This will override any error being returned and most probably replace it with nil. You'd need to keep the old error intact and only override it if it was nil.

karalabe · 2018-03-16T12:06:07Z

eth/downloader/statesync.go

-		if err := s.commit(false); err != nil {
-			return err
+		if err = s.commit(false); err != nil {
+			return


Please don't use naked returns, it's very hard to see what's being returned.

karalabe · 2018-03-16T12:06:10Z

eth/downloader/statesync.go

 				log.Warn("Node data write error", "err", err)
-				return err
+				return


Please don't use naked returns, it's very hard to see what's being returned.

karalabe · 2018-03-16T12:06:21Z

eth/downloader/statesync.go

 			}
 			req.peer.SetNodeDataIdle(len(req.response))
 		}
 	}
-	return s.commit(true)
+	return


Please don't use naked returns, it's very hard to see what's being returned.

Also the previous logic called s.commit(true) when it reached this point, but you replaced it with s.commit(false) (in the defer), so the behavior changes.

Sorry i can understand very well. I add the s.commit(true) in the defer because during the state node process procedure, some of the them can be invalid. If we return the error directly, we will lost the valid data committed before(which unreached the point will also be ignored).

* params, VERSION: v1.8.4 unstable * core/vm: Fixed typos in core/vm/interpreter.go * light: new CHT for ropsten (ethereum#16393) * whisper: fix issue in topic list copy (ethereum#16381) - Fixes ethereum#16271. What was appeneded was a pointer to an object that changes during the iteration. - The topic is allocated as a 4-byte array, fill partial topics with 0s. Partial topics are currently disabled, but would crash as they rely on the presence of byte number 3. * core/state: uniform parameter style (ethereum#16398) - Uniform code style. * core/state: rework dirty handling to avoid quadratic overhead * core/state: avoid linear overhead on journal dirty listing * travis.yml: remove sudo requirement for PPA and Azure purge builders (ethereum#16404) This is supposed to fix the FTP upload issue according to travis-ci/travis-ci#9391. * .gitattributes: enable solidity highlighting on github (ethereum#16425) * crypto/secp256k1: catch curve parameter parse errors (ethereum#16392) * core/state: avoid redundant addition to code size cache (ethereum#16427) * cmd/geth: remove relOracle variable (ethereum#16434) * eth: fix typos (ethereum#16414) * accounts/abi: improve test coverage (ethereum#16044) * README: change 'built in' to 'built-in' * core/types: remove String methods from struct types (ethereum#16205) Most of these methods did not contain all the relevant information inside the object and were not using a similar formatting type. Moreover, the existence of a suboptimal String method breaks usage with more advanced data dumping tools like go-spew. * Dockerfile: use non-privileged user account (ethereum#16052) * cmd/geth: update template for 'geth bug' command (ethereum#16350) * cmd/evm: print vm output when debug flag is on (ethereum#16326) * bmt: fix comment typos (ethereum#16461) * ethclient: remove empty object in newHeads subscription call (ethereum#16454) * compression/rle: delete RLE compression (ethereum#16468) * eth/downloader: flush state sync data before exit (ethereum#16280) * state: handle nil in journal dirties * core: add blockchain benchmarks * cmd/puppeth: fix node deploys for updated dockerfile user * Dockerfile.alltools: fix invalid command * common: delete StringToAddress, StringToHash (ethereum#16436) * common: delete StringToAddress, StringToHash These functions are confusing because they don't parse hex, but use the bytes of the string. This change removes them, replacing all uses of StringToAddress(s) by BytesToAddress([]byte(s)). * eth/filters: remove incorrect use of common.BytesToAddress * build: add -e and -X flags to get more information on ethereum#16433 (ethereum#16443) * core: remove stray account creations in state transition (ethereum#16470) The 'from' and 'to' methods on StateTransitions are reader methods and shouldn't have inadvertent side effects on state. It is safe to remove the check in 'from' because account existence is implicitly checked by the nonce and balance checks. If the account has non-zero balance or nonce, it must exist. Even if the sender account has nonce zero at the start of the state transition or no balance, the nonce is incremented before execution and the account will be created at that time. It is safe to remove the check in 'to' because the EVM creates the account if necessary. Fixes ethereum#15119 * travis, appveyor: bump to Go 1.10.1 * travis.yml: add TEST_PACKAGES to speed up swarm testing (ethereum#16456) This commit is meant to allow ecosystem projects such as ethersphere to minimize CI build times by specifying an environment variable with the packages to run tests on. If the environment variable isn't defined the build script will test all packages so this shouldn't affect the main go-ethereum repository. * les: add ps.lock.Unlock() before return (ethereum#16360) * core/state: fix bug in copy of copy State * core/state: fix ripemd-cornercase in Copy * core: txpool stable underprice drop order, perf fixes * miner: remove contention on currentMu for pending data retrievals (ethereum#16497) * ethdb: add leveldb write delay statistic (ethereum#16499) * eth/downloader: wait for all fetcher goroutines to exit before terminating (ethereum#16509) * cmd/clef, signer: initial poc of the standalone signer (ethereum#16154) * signer: introduce external signer command * cmd/signer, rpc: Implement new signer. Add info about remote user to Context * signer: refactored request/response, made use of urfave.cli * cmd/signer: Use common flags * cmd/signer: methods to validate calldata against abi * cmd/signer: work on abi parser * signer: add mutex around UI * cmd/signer: add json 4byte directory, remove passwords from api * cmd/signer: minor changes * cmd/signer: Use ErrRequestDenied, enable lightkdf * cmd/signer: implement tests * cmd/signer: made possible for UI to modify tx parameters * cmd/signer: refactors, removed channels in ui comms, added UI-api via stdin/out * cmd/signer: Made lowercase json-definitions, added UI-signer test functionality * cmd/signer: update documentation * cmd/signer: fix bugs, improve abi detection, abi argument display * cmd/signer: minor change in json format * cmd/signer: rework json communication * cmd/signer: implement mixcase addresses in API, fix json id bug * cmd/signer: rename fromaccount, update pythonpoc with new json encoding format * cmd/signer: make use of new abi interface * signer: documentation * signer/main: remove redundant option * signer: implement audit logging * signer: create package 'signer', minor changes * common: add 0x-prefix to mixcaseaddress in json marshalling + validation * signer, rules, storage: implement rules + ephemeral storage for signer rules * signer: implement OnApprovedTx, change signing response (API BREAKAGE) * signer: refactoring + documentation * signer/rules: implement dispatching to next handler * signer: docs * signer/rules: hide json-conversion from users, ensure context is cleaned * signer: docs * signer: implement validation rules, change signature of call_info * signer: fix log flaw with string pointer * signer: implement custom 4byte databsae that saves submitted signatures * signer/storage: implement aes-gcm-backed credential storage * accounts: implement json unmarshalling of url * signer: fix listresponse, fix gas->uint64 * node: make http/ipc start methods public * signer: add ipc capability+review concerns * accounts: correct docstring * signer: address review concerns * rpc: go fmt -s * signer: review concerns+ baptize Clef * signer,node: move Start-functions to separate file * signer: formatting * light: new CHTs (ethereum#16515) * params: release Geth v1.8.4 * VERSION, params: begin v1.8.5 release cycle * build: enable goimports and varcheck linters (ethereum#16446) * core/asm: remove unused condition (ethereum#16487) * cmd/utils: fix help template issue for subcommands (ethereum#16351) * rpc: clean up IPC handler (ethereum#16524) This avoids logging accept errors on shutdown and removes a bit of duplication. It also fixes some goimports lint warnings. * core/asm: accept uppercase instructions (ethereum#16531) * all: fix various typos (ethereum#16533) * fix typo * fix typo * fix typo * rpc: handle HTTP response error codes (ethereum#16500) * whisper/whisperv6: post returns the hash of sent message (ethereum#16495) * ethclient: add DialContext and Close (ethereum#16318) DialContext allows users to pass a Context object for cancellation. Close closes the underlying RPC connection. * vendor: update elastic/gosigar so that it compiles on OpenBSD (ethereum#16542) * eth/downloader: fix for Issue ethereum#16539 (ethereum#16546) * params: release Geth v1.8.5 - Dirty Derivative² * VERSION, params: begin Geth 1.8.6 release cycle * cmd/geth: update the copyright year in the geth command usage (ethereum#16537) * Revert "Dockerfile.alltools: fix invalid command" * Revert "cmd/puppeth: fix node deploys for updated dockerfile user" * Dockerfile: revert the user change PR that broke all APIs * Dockerfile: drop legacy discovery v5 port mappings * params: release v1.8.6 to fix docker images * VERSION, params: begin release cycle 1.8.7 * cmd/geth, mobile: add memsize to pprof server (ethereum#16532) * cmd/geth, mobile: add memsize to pprof server This is a temporary change, to be reverted before the next release. * cmd/geth: fix variable name * core/types: avoid duplicating transactions on changing signer (ethereum#16435) * core/state: cache missing storage entries (ethereum#16584) * cmd/utils: point users to --syncmode under DEPRECATED (ethereum#16572) Indicate that --light and --fast options are replaced by --syncmode * trie: remove unused `buf` parameter (ethereum#16583) * core, eth: fix tracer dirty finalization * travis.yml: remove obsolete brew-cask install * whisper: Golint fixes in whisper packages (ethereum#16637) * vendor: fix leveldb crash when bigger than 1 TiB * core: ensure local transactions aren't discarded as underpriced This fixes an issue where local transactions are discarded as underpriced when the pool and queue are full. * evm/main: use blocknumber from genesis * accounts: golint updates for this or self warning (ethereum#16627) * tests: golint fixes for tests directory (ethereum#16640) * trie: golint iterator fixes (ethereum#16639) * internal: golint updates for this or self warning (ethereum#16634) * core: golint updates for this or self warning (ethereum#16633) * build: Add ldflags -s -w when building aar Smaller size on mobile is always good. Might also solve our maven central upload problem * cmd/clef: documentation about setup (ethereum#16568) clef: documentation about setup * params: release geth 1.8.7 * Zero Block Reward Post Byzantium * Remove Difficulty Bomb * 1 Second Block * Lower Minimum Difficulty (#4) * 1 Second Blocks (#5) * Lower Minimum Difficulty * One Second Blocks * Lower minimum difficulty (#6) * Enable geth compile.solidity for rpc (#7)

eth/downloader: flush state data before exit

b3de537

rjl493456442 requested a review from karalabe as a code owner March 8, 2018 02:30

karalabe mentioned this pull request Mar 8, 2018

internal: fix downloader synchronising judgement #16238

Closed

karalabe requested changes Mar 16, 2018

View reviewed changes

karalabe added this to the 1.8.4 milestone Mar 16, 2018

trie, eth: minor fix

1a243af

fjl approved these changes Apr 9, 2018

View reviewed changes

fjl merged commit 1100e8b into ethereum:master Apr 9, 2018

jmank88 mentioned this pull request Apr 28, 2018

eth/downloader: flush state sync data before exit (#16280) gochain/gochain#143

Merged

hackmod pushed a commit to OpenCommunityCoin/go-esn that referenced this pull request Jul 9, 2018

eth/downloader: flush state sync data before exit (ethereum#16280)

dad6b00

quocneo mentioned this pull request Jul 31, 2018

Synchronization merges from Go-Ethereum haloplatform/go-haloplatform#14

Closed

mariameda pushed a commit to NiluPlatform/go-nilu that referenced this pull request Aug 23, 2018

eth/downloader: flush state sync data before exit (ethereum#16280)

2767e84

firmianavan pushed a commit to firmianavan/go-ethereum that referenced this pull request Aug 28, 2018

eth/downloader: flush state sync data before exit (ethereum#16280)

8d07062

KimKyungup mentioned this pull request May 21, 2020

Refine State migration klaytn/klaytn#520

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

eth/downloader: flush state data before exit #16280

eth/downloader: flush state data before exit #16280

rjl493456442 commented Mar 8, 2018

karalabe Mar 16, 2018

karalabe Mar 16, 2018

karalabe Mar 16, 2018

karalabe Mar 16, 2018

karalabe Mar 16, 2018

karalabe Mar 16, 2018

karalabe Mar 16, 2018

rjl493456442 Mar 16, 2018 •

edited

Loading

eth/downloader: flush state data before exit #16280

eth/downloader: flush state data before exit #16280

Conversation

rjl493456442 commented Mar 8, 2018

karalabe Mar 16, 2018

Choose a reason for hiding this comment

karalabe Mar 16, 2018

Choose a reason for hiding this comment

karalabe Mar 16, 2018

Choose a reason for hiding this comment

karalabe Mar 16, 2018

Choose a reason for hiding this comment

karalabe Mar 16, 2018

Choose a reason for hiding this comment

karalabe Mar 16, 2018

Choose a reason for hiding this comment

karalabe Mar 16, 2018

Choose a reason for hiding this comment

rjl493456442 Mar 16, 2018 • edited Loading

Choose a reason for hiding this comment

rjl493456442 Mar 16, 2018 •

edited

Loading