-
Notifications
You must be signed in to change notification settings - Fork 20.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core, ethclient/gethclient: improve flaky tests #25918
Changes from 6 commits
1a9ea2e
3b239aa
74b0207
e381087
d143c93
ac25046
f0c328f
bc0b494
fabac7e
b3c24b0
30fa91e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -856,9 +856,13 @@ func (bc *BlockChain) writeHeadBlock(block *types.Block) { | |
headBlockGauge.Update(int64(block.NumberU64())) | ||
} | ||
|
||
// Stop stops the blockchain service. If any imports are currently in progress | ||
// it will abort them using the procInterrupt. | ||
func (bc *BlockChain) Stop() { | ||
// stop stops the blockchain service. If any imports are currently in progress | ||
// it will abort them using the procInterrupt. This method stops all running | ||
// goroutines, but does not do all the post-stop work of persisting data. | ||
// OBS! It is generally recommended to use the Stop method! | ||
// This method has been exposed to allow tests to stop the blockchain while simulating | ||
// a crash. | ||
func (bc *BlockChain) stop() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe rename it to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah maybe.. Another alternative - how about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. or There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe |
||
if !atomic.CompareAndSwapInt32(&bc.running, 0, 1) { | ||
return | ||
} | ||
|
@@ -878,7 +882,12 @@ func (bc *BlockChain) Stop() { | |
// returned. | ||
bc.chainmu.Close() | ||
bc.wg.Wait() | ||
} | ||
|
||
// Stop stops the blockchain service. If any imports are currently in progress | ||
// it will abort them using the procInterrupt. | ||
func (bc *BlockChain) Stop() { | ||
bc.stop() | ||
// Ensure that the entirety of the state snapshot is journalled to disk. | ||
var snapBase common.Hash | ||
if bc.snaps != nil { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -247,6 +247,7 @@ func (snaptest *crashSnapshotTest) test(t *testing.T) { | |
// Pull the plug on the database, simulating a hard crash | ||
db := chain.db | ||
db.Close() | ||
chain.stop() | ||
|
||
// Start a new blockchain back up and see where the repair leads us | ||
newdb, err := rawdb.NewLevelDBDatabaseWithFreezer(snaptest.datadir, 0, 0, snaptest.datadir, "", false) | ||
|
@@ -388,15 +389,17 @@ func (snaptest *wipeCrashSnapshotTest) test(t *testing.T) { | |
SnapshotLimit: 256, | ||
SnapshotWait: false, // Don't wait rebuild | ||
} | ||
_, err = NewBlockChain(snaptest.db, config, snaptest.gspec, nil, snaptest.engine, vm.Config{}, nil, nil) | ||
tmp, err := NewBlockChain(snaptest.db, config, snaptest.gspec, nil, snaptest.engine, vm.Config{}, nil, nil) | ||
if err != nil { | ||
t.Fatalf("Failed to recreate chain: %v", err) | ||
} | ||
tmp.Stop() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original intention here is to simulate an unclean shutdown without proper stop. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, ok, then I should change it to
holiman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Simulate the blockchain crash. | ||
newchain, err = NewBlockChain(snaptest.db, nil, snaptest.gspec, nil, snaptest.engine, vm.Config{}, nil, nil) | ||
if err != nil { | ||
t.Fatalf("Failed to recreate chain: %v", err) | ||
} | ||
defer newchain.Stop() | ||
snaptest.verify(t, newchain, blocks) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugly. Use deferred stops instead, or put the test in a method so the defer happen sooner