You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "eolivelli (via GitHub)" <gi...@apache.org> on 2026/02/10 13:58:23 UTC
[PR] [CELEBORN-2263] Fix IndexOutOfBoundsException while reading from S3 [celeborn]
eolivelli opened a new pull request, #3600:
URL: https://github.com/apache/celeborn/pull/3600
### What changes were proposed in this pull request?
Properly pass the size of the array to the InputStream that feeds the flush.
### Why are the changes needed?
Because without this change if the array is bigger than the buffer, then the inputstream returns garbage, resulting in corrupted data on S3
### Does this PR resolve a correctness bug?
Yes
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Manual testing
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2263] Fix IndexOutOfBoundsException while reading from S3 [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #3600:
URL: https://github.com/apache/celeborn/pull/3600#issuecomment-3891268570
cc @TheodoreLx @RexXiong, who originally wrote and committed this code, could you please take a look?
@eolivelli, it's holiday season in China until the end of Feb, so replies from reviewers might be delayed.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2263] Fix IndexOutOfBoundsException while reading from S3 [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #3600:
URL: https://github.com/apache/celeborn/pull/3600#discussion_r2799177236
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/storage/FlushTask.scala:
##########
@@ -144,7 +144,7 @@ private[worker] class S3FlushTask(
override def flush(copyBytes: Array[Byte]): Unit = {
val readableBytes = buffer.readableBytes()
val bytes = convertBufferToBytes(buffer, copyBytes, readableBytes)
- val inputStream = new ByteArrayInputStream(bytes)
+ val inputStream = new ByteArrayInputStream(bytes, 0, readableBytes)
Review Comment:
this matches what you say in the PR description, and makes sense to me.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2263] Fix IndexOutOfBoundsException while reading from S3 [celeborn]
Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli commented on code in PR #3600:
URL: https://github.com/apache/celeborn/pull/3600#discussion_r2799576984
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/storage/FlushTask.scala:
##########
@@ -41,7 +41,7 @@ abstract private[worker] class FlushTask(
copyBytes: Array[Byte],
length: Int): Array[Byte] = {
if (copyBytes != null && copyBytes.length >= length) {
- buffer.readBytes(copyBytes, 0, length)
+ buffer.getBytes(buffer.readerIndex(), copyBytes, 0, length)
Review Comment:
the main difference is here:
https://github.com/apache/celeborn/blob/e168fd54906b23011a9ada8d27b564f25c6c9920/worker/src/main/scala/org/apache/celeborn/service/deploy/worker/storage/Flusher.scala#L120-L123
when it "returns" the buffer we use the "readableBytes" to tell MemoryManager that the buffer has been returned
without this fix the variable `diskBufferCounter` in MemoryManager keeps increasing and never goes down
https://github.com/apache/celeborn/blob/77a8921648c77a6fd073a09e07aa6f41a98153c1/worker/src/main/java/org/apache/celeborn/service/deploy/worker/memory/MemoryManager.java#L72
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2263] Fix IndexOutOfBoundsException while reading from S3 [celeborn]
Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli commented on PR #3600:
URL: https://github.com/apache/celeborn/pull/3600#issuecomment-3885571119
@xy2953396112 @FMX @SteNicholas can you please take a look ?
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2263] Fix IndexOutOfBoundsException while reading from S3 [celeborn]
Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong commented on PR #3600:
URL: https://github.com/apache/celeborn/pull/3600#issuecomment-3904668001
Merge to main(v0.7.0) and branch-0.6(v0.6.3)
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2263] Fix IndexOutOfBoundsException while reading from S3 [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #3600:
URL: https://github.com/apache/celeborn/pull/3600#discussion_r2799162328
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/storage/FlushTask.scala:
##########
@@ -41,7 +41,7 @@ abstract private[worker] class FlushTask(
copyBytes: Array[Byte],
length: Int): Array[Byte] = {
if (copyBytes != null && copyBytes.length >= length) {
- buffer.readBytes(copyBytes, 0, length)
+ buffer.getBytes(buffer.readerIndex(), copyBytes, 0, length)
Review Comment:
the only difference here is whether to update the reader/writer index? how does this help?
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2263] Fix IndexOutOfBoundsException while reading from S3 [celeborn]
Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli commented on PR #3600:
URL: https://github.com/apache/celeborn/pull/3600#issuecomment-3891660395
thank you @pan3793
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2263] Fix IndexOutOfBoundsException while reading from S3 [celeborn]
Posted by "RexXiong (via GitHub)" <gi...@apache.org>.
RexXiong closed pull request #3600: [CELEBORN-2263] Fix IndexOutOfBoundsException while reading from S3
URL: https://github.com/apache/celeborn/pull/3600
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-2263] Fix IndexOutOfBoundsException while reading from S3 [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #3600:
URL: https://github.com/apache/celeborn/pull/3600#discussion_r2799162328
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/storage/FlushTask.scala:
##########
@@ -41,7 +41,7 @@ abstract private[worker] class FlushTask(
copyBytes: Array[Byte],
length: Int): Array[Byte] = {
if (copyBytes != null && copyBytes.length >= length) {
- buffer.readBytes(copyBytes, 0, length)
+ buffer.getBytes(buffer.readerIndex(), copyBytes, 0, length)
Review Comment:
the only difference here is whether to update the reader/writer index?
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org