jtuglu-netflix commented on code in PR #17652:
URL: https://github.com/apache/druid/pull/17652#discussion_r1925885503
##########
processing/src/main/java/org/apache/druid/io/LimitedOutputStream.java:
##########
@@ -24,18 +24,19 @@
import java.io.IOException;
import java.io.OutputStream;
+import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Function;
/**
* An {@link OutputStream} that limits how many bytes can be written. Throws
{@link IOException} if the limit
- * is exceeded.
+ * is exceeded. *Not* thread-safe.
*/
public class LimitedOutputStream extends OutputStream
{
private final OutputStream out;
private final long limit;
private final Function<Long, String> exceptionMessageFn;
- long written;
+ AtomicLong written;
Review Comment:
I get your point, but it's best-effort fix against the worst-case race (if
used incorrectly) I mentioned above that results in an overflow until a switch
is made to a properly thread-safe solution for a byte-limited version of
`ByteArrayOutputStream`. This will involve more changes not related to this
bug-fix which I think be logically separated into another PR. I can switch
back, but I don't see the harm in keeping it.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
