[Tests] Migrate tests of WriterMemoryBufferTest to LazyMemorySegmentPoolTest

[wuchong]

Search before asking

• I searched in the issues and found nothing similar.

Motivation

WriterMemoryBuffer has been deprecated and is replaced by LazyMemorySegmentPool, we should move tests of WriterMemoryBufferTest into LazyMemorySegmentPoolTest and then remove the WriterMemoryBuffer.

Solution

No response

Anything else?

No response

Willingness to contribute

• I'm willing to submit a PR!

[michaelkoepf]

@wuchong

Currently, lazilyAllocatePages() is implemented as follows

private void lazilyAllocatePages(int required) {
    if (cachePages.isEmpty()) { // <- Q1
        int numPages = Math.min(required, perRequestPages); // <- Q2
        for (int i = 0; i < numPages; i++) {
            cachePages.add(MemorySegment.allocateHeapMemory(pageSize));
        }
    }
}

and PER_REQUEST_MEMORY_SIZE is validated as follows

checkArgument(
        PER_REQUEST_MEMORY_SIZE > pageSize, // <- Q3
        String.format(
                "Page size should be less than PER_REQUEST_MEMORY_SIZE. Page size is:"
                        + " %s KB, PER_REQUEST_MEMORY_SIZE is %s KB.",
                pageSize / 1024, PER_REQUEST_MEMORY_SIZE / 1024));

• [Q1] Shouldn't the condition be cachePages.size() < required to cover the case where we have cached pages but the number of cached pages is not sufficient to satisfy the allocation request?
• [Q2] What is the semantics of perRequestPages? My assumption was that it is the minimum number of pages that can be allocated to have a buffer of pages ready for subsequent requests. If my assumption is correct, shouldn't this be Math.max(required - cachePages.size(), perRequestPages)?
• [Q3] Shouldn't the condition be PER_REQUEST_MEMORY_SIZE >= pageSize (i.e., at least 1 full page)?

Additionally, I think that we may unnecessarily block in waitForSegment() in cases where the request is impossible to satisfy (i.e., maxPages < requiredPages). Couldn't we fail early using

if (maxPages < requiredPages) { // do not wait if the request cannot be satisfied
    throw new EOFException(
            String.format(
                    "Allocation request cannot be satisfied because the number of maximum available pages is "
                            + "exceeded. Available pages: %d. Request pages: %d",
                    this.maxPages, requiredPages));
}

as first statement in waitForSegment()?

You can find a preliminary version of the above suggested changes here.