samarthjain commented on code in PR #17652:
URL: https://github.com/apache/druid/pull/17652#discussion_r1925867649
##########
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:
If the class is not thread safe, then I don't see the point of using an
`AtomicLong` here. The hope is that someone would read the above documentation
and not share the stream among multiple threads.
##########
server/src/main/java/org/apache/druid/query/ResultLevelCachingQueryRunner.java:
##########
@@ -152,6 +153,8 @@ public void after(boolean isDone, Throwable thrown)
// The resultset identifier and its length is cached along
with the resultset
resultLevelCachePopulator.populateResults();
log.debug("Cache population complete for query %s",
query.getId());
+ } else { // thrown == null &&
!resultLevelCachePopulator.isShouldPopulate()
+ log.error("Failed (and recovered) to populate result level
cache for query %s", query.getId());
Review Comment:
The error message is a bit confusing. This block will be hit when
`!resultLevelCachePopulator.isShouldPopulate()` evaluates to true. So no
attempt would have been made to populate the result level cache. Also, if
thrown is null, why was there a failure?
--
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]
