Mas i1807 overflowqueues#1809
Conversation
Also don't log queues on a crash - avoid over-sized crash dumps
Have the queue backed to disk, so that beyond a certain size it overflows from memory to disk (and once the on disk part is consumed from the queue the files are removed).
Risk of misconfiguration leading to wiping of wrong data. Also starting a job may lead to the main process having its disk_log wiped.
Avoid enoent errors
Check for a soft overload on any vnode before reaping. This will add some delay - but reap has been show to potentially overload a cluster ... availability is more important than reap speed.
There is no catch for {error, mailbox_overload} should it occur - it should not as the mailbox check should prevent it. If it does, the reaper will crash (and restart without any reaps) - return to a safe known position.
|
The proposed change implements a disk-backed buffer for delete and reap request queues (as opposed to in-memory queues). It is a mitigation against said queues growing and consuming too much memory, and potentially causing OOM conditions on a node, observed when, for example, a node re-joins its cluster after substantial amount of data has been modified/deleted in the cluster, causing massive AAE activity with which the joining node is unable to cope. I can see all pieces in the flow and I find eunit tests with good coverage, which I can confirm are passing. One concern that I have is about stale disk_log files: I agree that "wiping of wrong data" is a valid argument to the contrary; but something still needs to be done to prevent stale files from appearing. What if Other than that, a couple of nits: no need to ++ strings, and lots of trailing whitespace. Bonus points for using a solution from stdlib (disk_log) instead of reinventing it! |
|
@hmmr With regards to the use of '++', is this a suggestion to use iolists instead, with the improved efficiency? |
The queue file generated, are still in UUID format, but the id now incorporates creation date. This should make it easier to detect and clean any garbage that might be accrued. One use case where '++' is used has also been removed (although a lists:flatten/1 was still required at the end in this case)
hmmr
left a comment
hmmr
left a comment
There was a problem hiding this comment.
I meant to say that ++ between strings is redundant. But that's not important.
* Bound reap/erase queues
Also don't log queues on a crash - avoid over-sized crash dumps
* Create generic behaviour for eraser/reaper
Have the queue backed to disk, so that beyond a certain size it overflows from memory to disk (and once the on disk part is consumed from the queue the files are removed).
* Stop cleaning folder
Risk of misconfiguration leading to wiping of wrong data. Also starting a job may lead to the main process having its disk_log wiped.
* Setup folders correctly
Avoid enoent errors
* Correct log
* Further log correction
* Correct cleaning of directories for test
* Switch to action/2
* Update eqc tests for refactoring of reaper/eraser
* Improve comments/API
* Pause reaper on overload
Check for a soft overload on any vnode before reaping. This will add some delay - but reap has been show to potentially overload a cluster ... availability is more important than reap speed.
There is no catch for {error, mailbox_overload} should it occur - it should not as the mailbox check should prevent it. If it does, the reaper will crash (and restart without any reaps) - return to a safe known position.
* Adjustments following review
The queue file generated, are still in UUID format, but the id now incorporates creation date. This should make it easier to detect and clean any garbage that might be accrued.
One use case where '++' is used has also been removed (although a lists:flatten/1 was still required at the end in this case)
# Conflicts:
# priv/riak_kv.schema
# rebar.config
# src/riak_kv_overflow_queue.erl
# src/riak_kv_reaper.erl
# src/riak_kv_replrtq_src.erl
# src/riak_kv_test_util.erl
2 participants
Alternative solution to the unbound queue problem (e.g. alternative to #1808).
Rather than simply curtailing when size of queue hits an arbitrary limit - overflow to disk at a limit instead.
This solution also refactors the reaper/eraser to use a common behaviour to remove duplication of code.