apache / apache/bookkeeper

Discuss the lifecycle of application data bytebuffer when adding new entries to ledger

Open
#2,138 8 comments 0 reactions 0 assignees View on GitHub
area/client priority/blocker triage/week-33 type/question
Dominant language
Java
Stars
2k
Forks
976
Avg merge
6d 15h
Merged PRs (30d)
7

Description

**QUESTION**

The issue [here](https://github.com/apache/bookkeeper/issues/2067) and the corresponding PR [here](https://github.com/apache/bookkeeper/commit/bd699e61d729f327b5de302089d11bc3dcadfeb8) made some deep changes as to how application level byte buffers are utilized and released by bookkeeper client.

With this change, application level data buffer should not be reused anytime and the complete lifecycle of managing the refcounts and release of memory should be left to bk client.
If byte buf is reused, it can potentially corrupt the entries (Qw - Qa), attributed to certain race conditions in the code, which usually surface only under high load. The commit hasn't reverted from the community. The commit offers reduction in GC cycles and CPU usage.

As discussed in slack group, we had some discussion along these lines.
```
With the BetyBuf (refcounted) API since 4.8 we stated clearly (and changed behavior) that the net effect of addEntry is a -1 on the refcount, in other words BK takes ownership of the ByteBuf and it is now the responsible for disposal.
In 4.7 it was not so clear
```

However concerns were raised regarding the usage of the API `public long addEntry(byte[] data, int offset, int length)`, in which the application expects only part of buffer to be written to ledger. If bk client releases the buffer, the application can run into unexpected issues. If application reuses the buffer, it can lead to data corruption.

The issue is to discuss proper handling of these byte buffers and managing their lifecycle. We should standardize and put it up in the doc.

@eolivelli @sijie @jvrao @reddycharan @nicmichael Thoughts welcome.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.