Skip to content

Write journal entry length prefix and payload in a single BufferedChannel write#4833

Open
merlimat wants to merge 1 commit into
apache:masterfrom
merlimat:journal-single-buffered-write
Open

Write journal entry length prefix and payload in a single BufferedChannel write#4833
merlimat wants to merge 1 commit into
apache:masterfrom
merlimat:journal-single-buffered-write

Conversation

@merlimat

Copy link
Copy Markdown
Contributor

Motivation

On the journal append path, each entry is written to the BufferedChannel as two separate calls — the 4-byte length prefix and then the payload:

bc.write(lenBuff);
bc.write(qe.entry);

Each write() acquires the channel monitor, so every journal entry pays two lock acquisitions on a monitor that is also contended by the force-write thread (forceWrite() takes the same lock to reset unpersistedBytes), plus two rounds of position/unpersisted-bytes accounting.

Changes

  • Add a BufferedChannel.write(ByteBuf src1, ByteBuf src2) overload that copies both buffers into the write buffer under a single lock acquisition, with a single position/unpersisted-bytes update. The two source buffers are passed as separate arguments — deliberately avoiding a CompositeByteBuf, which would add a per-entry allocation and component bookkeeping.
  • The existing single-buffer write() is refactored onto the same internal helpers, with identical behavior.
  • Journal now writes the length prefix and entry payload with one call.
  • SlowBufferedChannel (test injection hook) overrides the new method so the configured add-delay still applies.
  • New unit test testWriteTwoBuffers covers boundary crossings: payloads that fit, exactly fill, straddle the write-buffer capacity, and exceed it (multiple internal flushes in one call), verifying position() and the resulting file contents.

Verified: BufferedChannelTest (9/9, including the new test), BookieJournalTest, BookieWriteToJournalTest, BookieJournalForceTest, BookieJournalPageCacheFlushTest, BookieJournalNoSyncTest, BookieJournalMaxMemoryTest (30/30), and module checkstyle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant