SteNicholas commented on code in PR #3809:
URL: https://github.com/apache/celeborn/pull/3809#discussion_r3845278421
##########
worker/src/test/scala/org/apache/celeborn/service/deploy/worker/storage/TierWriterSuite.scala:
##########
@@ -333,4 +385,162 @@ class TierWriterSuite extends AnyFunSuite with
BeforeAndAfterEach {
assert(fileLen == 10240)
assert(localTierWriter.closed === true)
}
+
+ test("memory tier writer should preserve state on composite buffer capacity
overflow") {
Review Comment:
Could we add a regression test that exercises a real **pre-insertion OOM**?
This capacity-overflow path throws `IllegalArgumentException`, so the old
implementation’s OOM catch would not run and its counters would already remain
unchanged. The consolidation-OOM tests cover post-insertion failure, which the
old code already accounted for. As a result, the current tests can all pass
without the production fix. One deterministic option is to use a real
`CompositeByteBuf` with a `ByteBuf` wrapper that throws `OutOfMemoryError` from
`readerIndex()` during `newComponent`, then assert that the counters/file
length remain unchanged and the caller’s ref count is restored.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
