pymaker icon indicating copy to clipboard operation
pymaker copied to clipboard

Should not execute complete shutdown on "No new blocks received for 300 seconds"

Open ghost opened this issue 7 years ago • 1 comments

Whenever the "No new blocks received for 300 seconds, the keeper will terminate" occurs, the full graceful shutdown takes place now. It might not be the desired thing though, as the full shutdown procedure may involve for example cancelling orders on Oasis etc. But if we start cancelling them when the chain is not in sync, we have no way to see if our transactions got properly mined or not. In case of increasing gas price it will make the keeper unnecessarily raise the gas price too high.

You can see what I mean in the following example. The node stopped syncing for about half an hour, the market-maker-keeper detected it and started to cancel orders. But the cancellation transactions probably did not even propagate, but it kept raising the gas price which was a complete waste as the transactions didn't get mined until the node started syncing again:

2017-12-11 07:10:26,952 INFO     Keeper terminated
2017-12-11 07:10:26,952 INFO     Shutdown logic finished
2017-12-11 07:10:26,951 INFO     Transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) was successful (tx_hash=0x86...)
2017-12-11 06:42:03,122 INFO     Sent transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) with nonce=7432, gas=214207, gas_price=150000000000 (tx_hash=0x86...)
2017-12-11 06:41:03,200 INFO     Sent transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) with nonce=7432, gas=214207, gas_price=130000000000 (tx_hash=0x57...)
2017-12-11 06:40:03,265 INFO     Sent transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) with nonce=7432, gas=214207, gas_price=110000000000 (tx_hash=0x2b...)
2017-12-11 06:39:03,293 INFO     Sent transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) with nonce=7432, gas=214207, gas_price=90000000000 (tx_hash=0x5c...)
2017-12-11 06:38:03,280 INFO     Sent transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) with nonce=7432, gas=214207, gas_price=70000000000 (tx_hash=0xc5...)
2017-12-11 06:37:03,781 INFO     Sent transaction MatchingMarket('0x3aa927a97594c3ab7d7bf0d47c71c3877d1de4a1').kill([b'\x00...']) with nonce=7432, gas=214207, gas_price=50000000000 (tx_hash=0x60...)
2017-12-11 06:37:02,127 INFO     Executing keeper shutdown logic...
2017-12-11 06:37:02,126 INFO     Waiting for outstanding callback to terminate...
2017-12-11 06:37:01,389 INFO     Waiting for all threads to terminate...
2017-12-11 06:37:01,389 INFO     Shutting down the keeper
2017-12-11 06:37:01,389 CRITICAL No new blocks received for 300 seconds, the keeper will terminate

Ultimately, I think the keeper should have a choice how to configure the lifecycle. It should be able to bind to an emergency_shutdown event.

ghost avatar Dec 11 '17 08:12 ghost

This is painfully apparent when working on a local testchain. My use case was integration testing auction-keeper. Workaround is to run a script which records a meaningless transaction (wrapping 1ETH) every 4 minutes.

EdNoepel avatar Apr 26 '19 18:04 EdNoepel