jihoonson commented on a change in pull request #11294:
URL: https://github.com/apache/druid/pull/11294#discussion_r658470583
##########
File path:
server/src/main/java/org/apache/druid/segment/realtime/appenderator/Appenderators.java
##########
@@ -108,7 +108,28 @@ public static Appenderator createOffline(
boolean batchMemoryMappedIndex
)
{
- return new AppenderatorImpl(
+ if (batchMemoryMappedIndex) {
Review comment:
This changes the behavior of `batchMemoryMappedIndex` which is
documented. Luckily, this flag was added in
http://github.com/apache/druid/pull/11123, which is not included in any release
yet. So we should either fix the document for this config or not change the
behavior. So, thinking about the flag, do we really need it? Per your comment
below, I guess you want to have these flags for providing a workaround to users
just in case where some unknown bugs are found after release. If this is the
case, I understand why you want, but it doesn't seem to me like a right
direction. No bugs found in production environments for a certain period of
time doesn't necessarily mean that some particular code area is completely free
from bugs. Rather, it means no bugs have been found in that area yet. So, it's
always possible that we find new bugs after we think this code area is stable
and get rid of these flags. If you are worrying about potential bugs in this
change, I think we should rathe
r add more tests to get more confidence. If you are not confident enough, we
should plan out what kind of tests we need more and add them.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
