Open theoreticalbts opened 6 years ago
The blockchain directory should have a special store for saving reversible blocks on shutdown and re-pushing them on startup. Because it does not, the starting block in run (b) is less than the starting block in run (a)
I do not view this as a bug. This is 100% intentional. The block_db
implementation of BitShares 2.0 and early versions of Steemd included reversible blocks. It was considerably more buggy, especially during unclean shutdown. The block log is not perfect, but it is considerably more reliable than previous implementations. I recommend this behavior simply be a quirk of running a test net node and no additional development be done. It is obviously not an issue on main net. The only time in which it could be a problem is on a chain halt. The blockchain makes no guarantees about the inclusion of a particular block or transaction. Losing the head block due to reversibility on a halt is no worse than losing pending transactions.
The fact that run (c) started at block 1 means steemd apparently does not check for the situation where block_log exists but shared_memory.bin does not. One reasonable response is to automatically replay in this situation. Another reasonable response is to exit(1) if the user did not specify --replay-blockchain or --resync-blockchain. A less reasonable, but still "correct", response is to rm block_log and resync. A totally buggy, incorrect response is the current behavior, which is apparently to start overwriting block_log without deleting/truncating it.
I agree this should be changed. Instead of automatically replaying, let's print a message prompting for a replay and exit.
The fact that run (d) successfully replayed is, at first glance, not too surprising since presumably the new blocks are the same size. But consider the block times: Blocks 1-16 were irreversible in run (c), so they should have been written into the block log by run (c). But blocks after c's LIB (which cannot be greater than 41, since (c) only produced 41 blocks) can only come from run (b) or possibly run (a). Which means they would have earlier times. Which means run (d) should have failed due to non-increasing timestamps. Which it did not. Which means I can't explain why run (d) didn't fail during replay.
if( log_head_num < dpo.last_irreversible_block_num )
The new chain would have appended new blocks past 135 on the new chain. This is problematic because the block log would contain blocks from two different blockchains. However, the replay check for a non-existent shared_memory.bin
will prevent this from occurring.
I do not view this as a bug. This is 100% intentional. The block_db implementation of BitShares 2.0 and early versions of Steemd included reversible blocks. It was considerably more buggy, especially during unclean shutdown. The block log is not perfect, but it is considerably more reliable than previous implementations.
I'm aware of that. It sounds like you're thinking of an implementation like the following:
I agree such an implementation would need a lot of work to get the second step right, as there's currently no code which does that. And I agree that previous implementations are probably pretty buggy if they just write blocks and hope for the best, without bothering to worry about the tedious business of persisting the undo data structures.
Instead, I'm thinking more along the lines of:
This is much less error-prone. We're just setting up another source of blocks which follow the normal path. It has the effect of rebuilding the undo structure in memory without actually having to write the code to persist it to disk.
unclean shutdown
The current code doesn't work, at all, with unclean shutdown (i.e. power cut or SIGKILL
). In my experience, unclean shutdown always results in corruption which requires truncating the block log to a known-good prefix and replaying.
On exit, blocks are popped to LIB (as we currently do).
This is not what currently happens. We pop back to LIB on startup because requiring a pop to LIB on shutdown is going to be violated on an unclean shutdown. Because undo is write ahead, we are less likely to run into a situation where we cannot recover state based even on an incomplete undo state.
The current code doesn't work, at all, with unclean shutdown (i.e. power cut or SIGKILL). In my experience, unclean shutdown always results in corruption which requires truncating the block log to a known-good prefix and replaying.
This may be your experience, but it is not representative of mine or many of the node operators. To the best of my memory, the only scenario when we are not recoverable is when shared memory runs out of space. I've done many SIGKILLs and have successfully recovered. Perhaps OS X is better at writing files and persisting mmap than Linux?
steemd
produces blocks 1-145steemd
produces blocks 125-156 (expected, as first shutdown throws away non-irreversible blocks)rm -f shared_memory.bin
curl
to checkdatabase_api.get_dynamic_global_properties
, it returned{"head_block_number":37, "last_irreversible_block_num":16}
steemd --replay-blockchain
produces blocks 136-147That this sequence of events can occur reveals several bugs:
steemd
apparently does not check for the situation whereblock_log
exists butshared_memory.bin
does not. One reasonable response is to automatically replay in this situation. Another reasonable response is toexit(1)
if the user did not specify--replay-blockchain
or--resync-blockchain
. A less reasonable, but still "correct", response is torm block_log
and resync. A totally buggy, incorrect response is the current behavior, which is apparently to start overwritingblock_log
without deleting/truncating it.Without looking at the code, my best guess is that it's some race condition with the OS filesystem. So for reference, here is the environment (running on a manually provisioned AWS EC2 instance):
Here's the log:
In another terminal window, result of
get_dynamic_global_properties
during the second execution which produced from block 1: