pvary commented on a change in pull request #1941:
URL: https://github.com/apache/hive/pull/1941#discussion_r570026024



##########
File path: 
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java
##########
@@ -338,6 +338,24 @@ public void setupConfiguration(Configuration conf) {
       conf.setInt(MRJobConfig.REDUCE_MEMORY_MB, 512);
       conf.setInt(MRJobConfig.MR_AM_VMEM_MB, 128);
     }
+
+    // we want this shim to override these values only if the default value is 
set, otherwise there
+    // is a good chance that the user of the minicluster set another value 
intentionally
+    protected void overrideIntIfDefaultIsSet(Configuration conf, String key, 
int defaultVal,
+        int newVal) {
+      if (conf.getInt(key, defaultVal) == defaultVal) {
+        LOG.info("Hadoop23Shims overrides '{}' from {} to {}", key, 
defaultVal, newVal);
+        conf.setInt(key, newVal);
+      }

Review comment:
       Would it make sense to LOG even if we do not override?
   It took me several attempts to understand your comments in the description 
(I have kept looking for 256 in the logs), so either it is too early morning 
for me or some additional log messages would be good 😄 
   
   If I understand correctly this is test code only change so it would not be a 
problem if we add more log.
   
   Your thoughts? And BTW thanks for the patch!
   Peter




----------------------------------------------------------------
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]

Reply via email to