----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58085/#review170607 -----------------------------------------------------------
Thanks Aihua for the patch! It would be good to clear up this part of the code! Overall looks good, just some questions, nits. Sorry for the duplications, I was not able to remove them. When I tried it I created another one :( Thanks, Peter common/src/java/org/apache/hadoop/hive/common/LogUtils.java Lines 52 (patched) <https://reviews.apache.org/r/58085/#comment243486> Maybe we should rephrase the comment? ql/src/java/org/apache/hadoop/hive/ql/Driver.java Lines 1376-1380 (original) <https://reviews.apache.org/r/58085/#comment243479> With removing the these explicit writeOperationLog operations we remove the possibility to configure/fire these messages independently of the log4j log level (INFO/DEBUG/TRACE...), and only dependent of the Operation log level (EXECUTION/PERFORMANCE/...). Until this point it was possible to keep the log4j loglevel at ERROR, still if the operation log level was EXECUTE, we got this message. It might be intentional, or an accepted change, or I might missing here something. It is not easy to understand this whole OperationLog stuff :) ql/src/java/org/apache/hadoop/hive/ql/log/LogDivertAppender.java Lines 209 (patched) <https://reviews.apache.org/r/58085/#comment243481> We might be able to remove the HIVE_SERVER2_LOGGING_OPERATION_LOG_LOCATION from HiveConf, since we do not use it any more. Am I right? ql/src/java/org/apache/hadoop/hive/ql/log/LogDivertAppender.java Lines 209 (patched) <https://reviews.apache.org/r/58085/#comment243482> We might be able to remove the HIVE_SERVER2_LOGGING_OPERATION_LOG_LOCATION from HiveConf, since we do not use it any more. Am I right? ql/src/java/org/apache/hadoop/hive/ql/log/LogDivertAppender.java Lines 209 (patched) <https://reviews.apache.org/r/58085/#comment243484> We might be able to remove the HIVE_SERVER2_LOGGING_OPERATION_LOG_LOCATION from HiveConf, since we do not use it any more. Am I right? ql/src/java/org/apache/hadoop/hive/ql/log/LogDivertAppender.java Lines 209 (patched) <https://reviews.apache.org/r/58085/#comment243491> We might be able to remove the HIVE_SERVER2_LOGGING_OPERATION_LOG_LOCATION from HiveConf, since we do not use it any more. Am I right? service/src/java/org/apache/hive/service/cli/operation/Operation.java Lines 219-252 (original) <https://reviews.apache.org/r/58085/#comment243474> We do not want this nice sophisticated log file creation? We belive that the operation handle is created, and the appropriate error messages are printed by log4j? service/src/java/org/apache/hive/service/cli/operation/Operation.java Line 295 (original), 246 (patched) <https://reviews.apache.org/r/58085/#comment243476> I think it might be better to have a LogUtils.unregisterLoggingContext. What do you think? service/src/java/org/apache/hive/service/cli/operation/OperationManager.java Line 78 (original), 74 (patched) <https://reviews.apache.org/r/58085/#comment243487> What happens, when the operation log is turned off by default, but later for some session we turn it on? It seems to me, that the init is called, but we might have some unexpected results :) Like from now on we will have the routingAppender, and every session will start to log in EXECUTE level, since that is the default HIVE_SERVER2_LOGGING_OPERATION_LEVEL. Just a guess, I did not have time to try it out :) - Peter Vary On March 30, 2017, 4:54 p.m., Aihua Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58085/ > ----------------------------------------------------------- > > (Updated March 30, 2017, 4:54 p.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > ------- > > HIVE-16061: Some of console output is not printed to the beeline console > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/common/LogUtils.java > 01b2e7c2e0568eebde6af7fe9c1e359d7ec5e7e8 > ql/src/java/org/apache/hadoop/hive/ql/Driver.java > d981119d3f6eb8fba66bf7c16aee838280d1c969 > ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java > a596e92d8d67e7a96d8164de086c4f2eca0b0403 > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java > 1945163a0e2cfce53ee75c742143367ad23f97ed > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapredLocalTask.java > 591ea973f97913cad4cd7e96dd1ffc6a301b4bb1 > ql/src/java/org/apache/hadoop/hive/ql/log/LogDivertAppender.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java > 08d0544c49e6d2c2bbe473e8dfed4f38c1606ca7 > ql/src/java/org/apache/hadoop/hive/ql/session/OperationLog.java > 18216f25444a0b7867cbbf0ae0a5046a686b9e64 > > service/src/java/org/apache/hive/service/cli/operation/HiveCommandOperation.java > 8f08c2e2ad0bb73494b2cd23359af5594997ebcf > > service/src/java/org/apache/hive/service/cli/operation/LogDivertAppender.java > eaf1acbcfeb687eeebf1b3f7eed2099241fc46a2 > service/src/java/org/apache/hive/service/cli/operation/Operation.java > 11a820fae6bb6a5815e0efb113ec73a399b0b5bd > > service/src/java/org/apache/hive/service/cli/operation/OperationManager.java > 3f8f68e31fde0c8c578a1052b1f98b7a86a55089 > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java > f41092ed15ac8b88de72222e436abd80bdb8639b > > > Diff: https://reviews.apache.org/r/58085/diff/1/ > > > Testing > ------- > > Test is done locally. You can see the beeline output now. > > 0: jdbc:hive2://localhost:10000> select t1.key from src t1 join src t2 on > t1.key=t2.key limit 10; > INFO : Compiling > command(queryId=axu_20170330125216_0de3bbf7-60f5-476d-b7eb-9861891d2961): > select t1.key from src t1 join src t2 on t1.key=t2.key limit 10 > INFO : Semantic Analysis Completed > INFO : Returning Hive schema: Schema(fieldSchemas:[FieldSchema(name:t1.key, > type:string, comment:null)], properties:null) > INFO : Completed compiling > command(queryId=axu_20170330125216_0de3bbf7-60f5-476d-b7eb-9861891d2961); > Time taken: 0.219 seconds > INFO : Concurrency mode is disabled, not creating a lock manager > INFO : Executing > command(queryId=axu_20170330125216_0de3bbf7-60f5-476d-b7eb-9861891d2961): > select t1.key from src t1 join src t2 on t1.key=t2.key limit 10 > WARN : Hive-on-MR is deprecated in Hive 2 and may not be available in the > future versions. Consider using a different execution engine (i.e. spark, > tez) or using Hive 1.X releases. > INFO : Query ID = axu_20170330125216_0de3bbf7-60f5-476d-b7eb-9861891d2961 > INFO : Total jobs = 1 > INFO : Starting task [Stage-4:MAPREDLOCAL] in serial mode > INFO : Execution completed successfully > INFO : MapredLocal task succeeded > INFO : Launching Job 1 out of 1 > INFO : Starting task [Stage-3:MAPRED] in serial mode > INFO : Number of reduce tasks is set to 0 since there's no reduce operator > INFO : Starting Job = job_local1894165710_0002, Tracking URL = > http://localhost:8080/ > INFO : Kill Command = > /Users/axu/Documents/workspaces/tools/hadoop/hadoop-2.6.0/bin/hadoop job > -kill job_local1894165710_0002 > INFO : Hadoop job information for Stage-3: number of mappers: 0; number of > reducers: 0 > INFO : 2017-03-30 12:52:21,788 Stage-3 map = 0%, reduce = 0% > ERROR : Ended Job = job_local1894165710_0002 with errors > ERROR : FAILED: Execution Error, return code 2 from > org.apache.hadoop.hive.ql.exec.mr.MapRedTask > INFO : MapReduce Jobs Launched: > INFO : Stage-Stage-3: HDFS Read: 0 HDFS Write: 0 FAIL > INFO : Total MapReduce CPU Time Spent: 0 msec > > > Thanks, > > Aihua Xu > >
