Don't suspend writes based on batch size. Instead, log if it seems to be too big

[mtrippled]

High Level Overview of Change

Removes a limit that blocks nodestore writes. This delays consensus because each new ledger is persisted during consensus processing.

Context of Change

This removes the check that limits write batch size and blocks when the limit is reached. It also adds logging to indicate when the limit is exceeded. The trade-off is that the pending writes consume memory until they are persisted, so memory can increase without the limit. But since blocking writes result in consensus instability, the existing behavior is a proactive pessimization. Ultimately, pending writes either increase due to increased input (transaction volume), or because of I/O slowdown. If the IO and memory subsystem can't handle the throughput then the process will be unstable regardless.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (non-breaking change that only restructures code)
• Tests (You added tests for code that already exists, or your new feature included in this PR)
• Documentation Updates
• Performance
• Release

[HowardHinnant]

This looks fine to me, but I have a question: It looks like the code just adds logging. I don't see code that does the other part of the title: "Don't suspend writes based on batch size." What am I missing?

[mtrippled]

@HowardHinnant Here's the logic change.

[mtrippled]

This looks fine to me, but I have a question: It looks like the code just adds logging. I don't see code that does the other part of the title: "Don't suspend writes based on batch size." What am I missing?

fixed

[seelabs]

👍 This patch fixes a potential deadlock as well. Needs to be reformatted before merging though. Nice job Mark!

[intelliot]

@sophiax851 this may require perf signoff

[intelliot]

Internal tracker: RPFC-80

[intelliot]

May be replaced by #4882

[intelliot]

• What happens when we want to write faster than NuDB is able to write? We have to do something to slow down the write rate.
• Could we just slow down the writes since there is no rush?
• Yes, but we don't want to block all threads writing to NuDB. We could add a priority and make online_delete low priority. (That is the only thing that really generates surges of writes; also consider: fetching ledger history; closing a new ledger is a very small surge)
  • If buffer is somewhat full, pause the low priority writes.
  • Need to expose the "progress" of online_delete since it will take more time.
• Need to bound memory use so it's not infinite. Current approach allows writes to grow at a certain rate and if writes exceed the rate, we slow down all code that tries to write. It would slow down online_delete which is something we should do; but that would also slow down high-priority code which is advancing the ledger.
• NuDB tracks its own bytes/sec or objects/sec write rate and can tell if writes are coming in faster than that. It can measure what % of the server's capacity is being used. What should we do with that information? We can slow something down, but what?

[intelliot]

#4503 writes asynchronously to nudb with a background thread, but it has an upper limit of records that can be in the queue to be written. When reached, writes are blocked. Online deletion copies a ledger and ensures that it writes the entire ledger to nudb. This amount of data being written likely causes further writes to be blocked until the data is written. This includes persisting new records for each new ledger produced, which is synchronous for consensus. This causes desyncs. This PR (#4762) is expected to fix this.

If #4882 (which just reverts #4503) is merged, then #4503+#4762 (combined) can still be considered for testing+merging in the future.

[intelliot]

note via @mtrippled : there is no reason to merge this PR. We can consider #4503 (comment) - tweaking NuDB's write buffer is probably better design. We can design micro-benchmarks to show how it affects NuDB itself. Write prioritization is too much complexity, but if NuDB could write faster, then that was the whole point of #4503.