HPCC4J-606 DFSClient Allow ReadBuffer size to be set #788
Conversation
- Created new CircularByteBuffer class - Created CircularByteBufferTest - Made changes to allow read buffer size to be smaller than the read request size Signed-off-by: James McMullan James.McMullan@lexisnexis.com
|
Jira Issue: https://hpccsystems.atlassian.net/browse/HPCC4J-606 Jirabot Action Result: |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| throw new IOException("Not enough bytes available to skip"); | ||
| } | ||
|
|
||
| readPos += n; |
Check failure
Code scanning / CodeQL
Implicit narrowing conversion in compound assignment High
There was a problem hiding this comment.
let's take care of this alert
| @@ -0,0 +1,217 @@ | |||
| /******************************************************************************* | |||
| * HPCC SYSTEMS software Copyright (C) 2024 HPCC Systems®. | |||
|
|
||
| public CircularByteBuffer(int bufferSize) | ||
| { | ||
| buffer = new byte[bufferSize]; |
There was a problem hiding this comment.
are 0 and negative ints ok here?
| buffer = new byte[bufferSize]; | ||
| } | ||
|
|
||
| public int getCurrentNumberOfBytes() |
There was a problem hiding this comment.
what is the meaning of current bytes? I assume this is the number of occupied bytes ? is there a more self documenting name for the method and the member?
| return getSpace() > 0; | ||
| } | ||
|
|
||
| public int getSpace() |
| { | ||
| if (currentNumberOfBytes + length > buffer.length) | ||
| { | ||
| throw new IOException("Not enough space available"); |
There was a problem hiding this comment.
Who is the target of the message, and will this be enough information to trivially identify the problem, and formulate a solution?
| @@ -1190,28 +1179,6 @@ private void readDataInFetch() | |||
| // Loop here while data is being consumed quickly enough | |||
| while (remainingDataInCurrentRequest > 0) | |||
| { | |||
| if (CompileTimeConstants.PROFILE_CODE) | |||
There was a problem hiding this comment.
what replaces this removed logic? L1219?
| dis.readFully(this.readBuffer, currentBufferLen, bytesToRead); | ||
| synchronized (readBuffer) | ||
| { | ||
| // int contiguousCapacity = readBuffer.getContiguousSpace(); |
There was a problem hiding this comment.
do we plan to remove the comments?
| { | ||
| availBytes = readBuffer.getCurrentNumberOfBytes(); | ||
| } | ||
|
|
||
| // Do the check for closed first here to avoid data races |
There was a problem hiding this comment.
does this old comment mean you shouldn't getcurrentnumberofbytes first?
| @@ -0,0 +1,163 @@ | |||
| /******************************************************************************* | |||
| * HPCC SYSTEMS software Copyright (C) 2024 HPCC Systems®. | |||
| import org.junit.experimental.categories.Category; | ||
|
|
||
| @Category(org.hpccsystems.commons.annotations.RemoteTests.class) | ||
| @FixMethodOrder(MethodSorters.NAME_ASCENDING) |
There was a problem hiding this comment.
just confirming we need to execute the tests in alphabetical order
Signed-off-by: James McMullan James.McMullan@lexisnexis.com
Type of change:
Checklist:
Testing: