During file rotation and WAL shutdown, there was a race condition between users
of an autofile and its termination. To fix this, ensure operations on an
autofile are properly synchronized, and report errors when attempting to use an
autofile after it was closed.
Notably:
- Simplify the cancellation protocol between signal and Close.
- Exclude writers to an autofile during rotation.
- Add documentation about what is going on.
There is a lot more that could be improved here, but this addresses the more
obvious races that have been panicking unit tests.
## Description
Internalize some libs. This reduces the amount ot public API tendermint is supporting. The moved libraries are mainly ones that are used within Tendermint-core.
* libs/common: refactor libs common 3
- move nil.go into types folder and make private
- move service & baseservice out of common into service pkg
ref #4147
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* add changelog entry
As per #3043, this adds a ticker to sync the WAL every 2s while the WAL is running.
* Flush WAL every 2s
This adds a ticker that flushes the WAL every 2s while the WAL is
running. This is related to #3043.
* Fix spelling
* Increase timeout to 2mins for slower build environments
* Make WAL sync interval configurable
* Add TODO to replace testChan with more comprehensive testBus
* Remove extraneous debug statement
* Remove testChan in favour of using system time
As per
https://github.com/tendermint/tendermint/pull/3300#discussion_r255886586,
this removes the `testChan` WAL member and replaces the approach with a
system time-oriented one. In this new approach, we keep track of the
system time at which each flush and periodic flush successfully
occurred.
The naming of the various functions is also updated here to be more
consistent with "flushing" as opposed to "sync'ing".
* Update naming convention and ensure lock for timestamp update
* Add Flush method as part of WAL interface
Adds a `Flush` method as part of the WAL interface to enforce the idea
that we can manually trigger a WAL flush from outside of the WAL. This
is employed in the consensus state management to flush the WAL prior to
signing votes/proposals, as per https://github.com/tendermint/tendermint/issues/3043#issuecomment-453853630
* Update CHANGELOG_PENDING
* Remove mutex approach and replace with DI
The dependency injection approach to dealing with testing concerns could
allow similar effects to some kind of "testing bus"-based approach. This
commit introduces an example of this, where instead of relying on
(potentially fragile) timing of things between the code and the test, we
inject code into the function under test that can signal the test
through a channel.
This allows us to avoid the `time.Sleep()`-based approach previously
employed.
* Update comment on WAL flushing during vote signing
Co-Authored-By: thanethomson <connect@thanethomson.com>
* Simplify flush interval definition
Co-Authored-By: thanethomson <connect@thanethomson.com>
* Expand commentary on WAL disk flushing
Co-Authored-By: thanethomson <connect@thanethomson.com>
* Add broken test to illustrate WAL sync test problem
Removes test-related state (dependency injection code) from the WAL data
structure and adds test code to illustrate the problem with using
`WALGenerateNBlocks` and `wal.SearchForEndHeight` to test periodic
sync'ing.
* Fix test error messages
* Use WAL group buffer size to check for flush
A function is added to `libs/autofile/group.go#Group` in order to return
the size of the buffered data (i.e. data that has not yet been flushed
to disk). The test now checks that, prior to a `time.Sleep`, the group
buffer has data in it. After the `time.Sleep` (during which time the
periodic flush should have been called), the buffer should be empty.
* Remove config root dir removal from #3291
* Add godoc for NewWAL mentioning periodic sync
* node: decrease retry conn timeout in test
Should fix#3256
The retry timeout was set to the default, which is the same as the
accept timeout, so it's no wonder this would fail. Here we decrease the
retry timeout so we can try many times before the accept timeout.
* p2p: increase handshake timeout in test
This fails sometimes, presumably because the handshake timeout is so low
(only 50ms). So increase it to 1s. Should fix#3187
* privval: fix race with ping. closes#3237
Pings happen in a go-routine and can happen concurrently with other
messages. Since we use a request/response protocol, we expect to send a
request and get back the corresponding response. But with pings
happening concurrently, this assumption could be violated. We were using
a mutex, but only a RWMutex, where the RLock was being held for sending
messages - this was to allow the underlying connection to be replaced if
it fails. Turns out we actually need to use a full lock (not just a read
lock) to prevent multiple requests from happening concurrently.
* node: fix test name. DelayedStop -> DelayedStart
* autofile: Wait() method
In the TestWALTruncate in consensus/wal_test.go we remove the WAL
directory at the end of the test. However the wal.Stop() does not
properly wait for the autofile group to finish shutting down. Hence it
was possible that the group's go-routine is still running when the
cleanup happens, which causes a panic since the directory disappeared.
Here we add a Wait() method to properly wait until the go-routine exits
so we can safely clean up. This fixes#2852.
There's a time window after we call RotateFile() where autofile#index+1
does not exist. It will be created during the next call to Write(). BUT
if somebody calls NewReader() before Write(), it will fail with "open
/tmp/wal#index+1/wal: no such file or directory"
We must create file (either by calling gr.Head.openFile() or directly)
during NewReader() to ensure read calls succeed.
Closes#2538
* fix Group.RotateFile need call Flush() before rename. #2428
* fix some review issue. #2428
refactor Group's config: replace setting member with initial option
* fix a handwriting mistake
* fix a time window error between rename and write.
* fix a syntax mistake.
* change option name Get_ to With_
* fix review issue
* fix review issue
1) no need to stop the ticker in createTestGroup() method
2) now there is a symmetry - we start the ticker in OnStart(), we stop it
in OnStop()
Refs #2072
short: flushing the bufio buffer is not enough to ensure data
consistency.
long:
Saving an entry to the WAL calls writeLine to append data to the
autofile group backing the WAL, then calls group.Flush() to flush that
data to persistent storage. group.Flush() in turn proxies to
headBuf.flush(), flushing the active bufio.BufferedWriter. However,
BufferedWriter wraps a Writer, not another BufferedWriter, and the way
it flushes is by calling io.Writer.Write() to clear the BufferedWriter's
buffer. The io.Writer we're wrapping here is AutoFile, whose Write
method calls os.File.Write(), performing an unbuffered write to the
operating system, where, I assume, it sits in the OS buffers awaiting
sync. This means that Wal.Save does not, in fact, ensure the saved
operation is synced to disk before returning.
Sam Kleinman
M. J. Fromberger
rene
Marko
Erik Grinaker
zjubfd
Anton Kaliaev
Thane Thomson
Ethan Buchman
goolAdapter
Dev Ojha
Zach Ramsay
Thomas Corbière
Jae Kwon