bxfjb commented on issue #8017:
URL: https://github.com/apache/rocketmq/issues/8017#issuecomment-2056779446
> > > `ByteStreams.toByteArray()` creates large array in memory and is
unnecessary. It seems `FileChannel.transferFrom()` with NIO channel would be
more efficient for writing streams to a file.
> >
> >
> > Thx for reply, I tried `FileChannel.transferFrom()` but find out some
unittest failed:
> > ```
> > @Test
> > public void consumeQueueTest() throws ClassNotFoundException,
NoSuchMethodException {
> > ...
> > ByteBuffer cqItem1 = fileSegment.read(initPosition, unitSize);
> > Assert.assertEquals(baseOffset, cqItem1.getLong()); // failed
> > ...
> > }
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > > java.lang.AssertionError:
> > > Expected :1000
> > > Actual :0
> >
> >
> > It turns out the cq data was not flush correctly by debugging, I wonder
if the way I use `FileChannel.transferFrom()` is wrong: Before:
> > ```
> > byte[] byteArray = ByteStreams.toByteArray(inputStream);
> > writeFileChannel.position(position);
> > ByteBuffer buffer = ByteBuffer.wrap(byteArray);
> > while (buffer.hasRemaining()) {
> > writeFileChannel.write(buffer);
> > }
> > writeFileChannel.force(true);
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > After:
> > ```
> > ReadableByteChannel readableByteChannel =
Channels.newChannel(inputStream);
> > writeFileChannel.transferFrom(readableByteChannel,
position, length);
> > writeFileChannel.force(true);
> > ```
>
> It seems the modification is not equivalent.
`writeFileChannel.position(position)` has side effect, making
`writeFileChannel` written from `position`. But
`writeFileChannel.transferFrom()` exits when `position` is larger than the size
of `writeFileChannel`.
>
> You might extend the file of `writeFileChannel` to `position` and also
check if the transferred bytes is equal to `length`.
Agree with you. But then I was wondering if the failed unittest necessary,
it seems acting like:
- set a empty file's commitPosition to 100
- append some mocked ConsumeQueue Buffer to it
- `commitPosition` is larger than the size of `writeFileChannel`, commit
failed
Is it acceptable to remove step 1 in `consumeQueueTest`? @lizhimins 'cause
the scenario seems nonexistent in production.
Furthermore, `FileChannel.read()` has the same problem with
`FileChannel.write()`, but to return a `ByteBuffer`, calling it looks like
inevitable. I didn't find out how to use `FileChannel.transferTo()` to replace
it completely. I'd like to use `MappedByteBuffer` as a alternative.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
