You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by vvysotskyi <gi...@git.apache.org> on 2017/09/01 10:22:50 UTC
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
GitHub user vvysotskyi opened a pull request:
https://github.com/apache/drill/pull/930
DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender by default
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vvysotskyi/drill DRILL-5761
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/930.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #930
----
commit be17b285747b0ca867b6a3733c17c1b8187c7a6d
Author: Volodymyr Vysotskyi <vv...@gmail.com>
Date: 2017-08-31T12:52:43Z
DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender by default
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Posted by priteshm <gi...@git.apache.org>.
Github user priteshm commented on the issue:
https://github.com/apache/drill/pull/930
@vvysotskyi is it possible to change the port number for Lilith to avoid the conflict?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/930
@paul-rogers, @vrozov, thanks for the review. Squashed commits and rebased onto master.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi closed the pull request at:
https://github.com/apache/drill/pull/930
---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on the issue:
https://github.com/apache/drill/pull/930
Holding of on the merge waiting for review by @vrozov.
---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on the issue:
https://github.com/apache/drill/pull/930
I have some concern about this PR.
AFASIK, some of Drill developers use Lilith to debug issues when running individual unit testcases. If remove it, does it mean people has to manually reverse this PR, in order to get log in Lilith?
If you shutdown Lilith application while running unit test suite, does it hang as well?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138102613
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,111 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
+ information regarding copyright ownership. The ASF licenses this file to
+ You under the Apache License, Version 2.0 (the "License"); you may not use
+ this file except in compliance with the License. You may obtain a copy of
+ the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<configuration>
+
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender name="SOCKET" class="de.huxhorn.lilith.logback.appender.ClassicMultiplexSocketAppender">
+ <Compressing>true</Compressing>
+ <ReconnectionDelay>10000</ReconnectionDelay>
+ <IncludeCallerData>true</IncludeCallerData>
+ <RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ </appender>
+ </then>
+ </if>
+
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <!-- encoders are assigned the type
+ ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <!--
+ <appender name="FILE" class="ch.qos.logback.core.rolling.RollingFileAppender">
+ <file>${log.path}</file>
+ <rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
+ <fileNamePattern>${log.path}.%i</fileNamePattern>
+ <minIndex>1</minIndex>
+ <maxIndex>10</maxIndex>
+ </rollingPolicy>
+
+ <triggeringPolicy class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
+ <maxFileSize>100MB</maxFileSize>
+ </triggeringPolicy>
+ <encoder>
+ <pattern>%date{ISO8601} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+ -->
+
+ <logger name="org.apache.drill" additivity="false">
+ <level value="debug"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+<!-- <appender-ref ref="FILE" />-->
+ </logger>
+
+ <logger name="query.logger" additivity="false">
+ <level value="info"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+ </logger>
+
+ <!--
+ <logger name="io.netty" additivity="false">
+ <level value="debug"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+ </logger>
+ -->
+
+ <logger name="org.apache.hadoop" additivity="false">
+ <level value="info"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+<!-- <appender-ref ref="FILE" /> -->
+ </logger>
+
+ <logger name="com.mapr" additivity="false">
--- End diff --
Initially, I assumed that it would be useful for those, who work with the mapr-format-plugin. I agree with you that we should delete this logger.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r137938284
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,111 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
+ information regarding copyright ownership. The ASF licenses this file to
+ You under the Apache License, Version 2.0 (the "License"); you may not use
+ this file except in compliance with the License. You may obtain a copy of
+ the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<configuration>
+
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender name="SOCKET" class="de.huxhorn.lilith.logback.appender.ClassicMultiplexSocketAppender">
+ <Compressing>true</Compressing>
+ <ReconnectionDelay>10000</ReconnectionDelay>
+ <IncludeCallerData>true</IncludeCallerData>
+ <RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ </appender>
+ </then>
+ </if>
+
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <!-- encoders are assigned the type
+ ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <!--
+ <appender name="FILE" class="ch.qos.logback.core.rolling.RollingFileAppender">
+ <file>${log.path}</file>
+ <rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
+ <fileNamePattern>${log.path}.%i</fileNamePattern>
+ <minIndex>1</minIndex>
+ <maxIndex>10</maxIndex>
+ </rollingPolicy>
+
+ <triggeringPolicy class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
+ <maxFileSize>100MB</maxFileSize>
+ </triggeringPolicy>
+ <encoder>
+ <pattern>%date{ISO8601} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+ -->
+
+ <logger name="org.apache.drill" additivity="false">
+ <level value="debug"/>
--- End diff --
Please put the test debug level at `ERROR`. I have a copy of `logback-test.xml` that I copy into each working branch that sets the level to `ERROR`. Then, specific tests set more detailed logging as needed. Otherwise, the console is bombarded with unwanted logging messages making it very hard to find those of interest.
---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/930
Yes, we can change hiveserver2 port #, but I think that it would be better to disable Lilith by default since hiveserver2 port number may be used in the configs of other applications and it would be cumbersome to change all that configs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r137938300
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,111 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
+ information regarding copyright ownership. The ASF licenses this file to
+ You under the Apache License, Version 2.0 (the "License"); you may not use
+ this file except in compliance with the License. You may obtain a copy of
+ the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<configuration>
+
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender name="SOCKET" class="de.huxhorn.lilith.logback.appender.ClassicMultiplexSocketAppender">
+ <Compressing>true</Compressing>
+ <ReconnectionDelay>10000</ReconnectionDelay>
+ <IncludeCallerData>true</IncludeCallerData>
+ <RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ </appender>
+ </then>
+ </if>
+
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <!-- encoders are assigned the type
+ ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <!--
+ <appender name="FILE" class="ch.qos.logback.core.rolling.RollingFileAppender">
+ <file>${log.path}</file>
+ <rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
+ <fileNamePattern>${log.path}.%i</fileNamePattern>
+ <minIndex>1</minIndex>
+ <maxIndex>10</maxIndex>
+ </rollingPolicy>
+
+ <triggeringPolicy class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
+ <maxFileSize>100MB</maxFileSize>
+ </triggeringPolicy>
+ <encoder>
+ <pattern>%date{ISO8601} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+ -->
+
+ <logger name="org.apache.drill" additivity="false">
+ <level value="debug"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+<!-- <appender-ref ref="FILE" />-->
+ </logger>
+
+ <logger name="query.logger" additivity="false">
+ <level value="info"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+ </logger>
+
+ <!--
+ <logger name="io.netty" additivity="false">
+ <level value="debug"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+ </logger>
+ -->
+
+ <logger name="org.apache.hadoop" additivity="false">
+ <level value="info"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+<!-- <appender-ref ref="FILE" /> -->
+ </logger>
+
+ <logger name="com.mapr" additivity="false">
--- End diff --
Should this be in Apache Drill? Only the MapR profile pulls in MapR code... It is not clear that tests that happen to use the MapR profile want debug level logging from this subsystem. Better to use the `LogFixture` to set more detailed logging in those tests that need it.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138103031
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,111 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
+ information regarding copyright ownership. The ASF licenses this file to
+ You under the Apache License, Version 2.0 (the "License"); you may not use
+ this file except in compliance with the License. You may obtain a copy of
+ the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<configuration>
+
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender name="SOCKET" class="de.huxhorn.lilith.logback.appender.ClassicMultiplexSocketAppender">
+ <Compressing>true</Compressing>
+ <ReconnectionDelay>10000</ReconnectionDelay>
+ <IncludeCallerData>true</IncludeCallerData>
+ <RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ </appender>
+ </then>
+ </if>
+
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <!-- encoders are assigned the type
+ ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <!--
+ <appender name="FILE" class="ch.qos.logback.core.rolling.RollingFileAppender">
+ <file>${log.path}</file>
+ <rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
+ <fileNamePattern>${log.path}.%i</fileNamePattern>
+ <minIndex>1</minIndex>
+ <maxIndex>10</maxIndex>
+ </rollingPolicy>
+
+ <triggeringPolicy class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
+ <maxFileSize>100MB</maxFileSize>
+ </triggeringPolicy>
+ <encoder>
+ <pattern>%date{ISO8601} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+ -->
+
+ <logger name="org.apache.drill" additivity="false">
+ <level value="debug"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+<!-- <appender-ref ref="FILE" />-->
+ </logger>
+
+ <logger name="query.logger" additivity="false">
+ <level value="info"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+ </logger>
+
+ <!--
+ <logger name="io.netty" additivity="false">
+ <level value="debug"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+ </logger>
+ -->
+
+ <logger name="org.apache.hadoop" additivity="false">
+ <level value="info"/>
--- End diff --
Thanks for pointing this, already removed.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138407993
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,111 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
+ information regarding copyright ownership. The ASF licenses this file to
+ You under the Apache License, Version 2.0 (the "License"); you may not use
+ this file except in compliance with the License. You may obtain a copy of
+ the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<configuration>
+
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender name="SOCKET" class="de.huxhorn.lilith.logback.appender.ClassicMultiplexSocketAppender">
+ <Compressing>true</Compressing>
+ <ReconnectionDelay>10000</ReconnectionDelay>
+ <IncludeCallerData>true</IncludeCallerData>
+ <RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ </appender>
+ </then>
+ </if>
+
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <!-- encoders are assigned the type
+ ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <!--
+ <appender name="FILE" class="ch.qos.logback.core.rolling.RollingFileAppender">
+ <file>${log.path}</file>
+ <rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
+ <fileNamePattern>${log.path}.%i</fileNamePattern>
+ <minIndex>1</minIndex>
+ <maxIndex>10</maxIndex>
+ </rollingPolicy>
+
+ <triggeringPolicy class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
+ <maxFileSize>100MB</maxFileSize>
+ </triggeringPolicy>
+ <encoder>
+ <pattern>%date{ISO8601} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+ -->
+
+ <logger name="org.apache.drill" additivity="false">
+ <level value="debug"/>
--- End diff --
Can this logger be defined between line 21 and 22 in a single if condition?
---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/930
Unit tests hang when Lilith application is not running, but when `ClassicMultiplexSocketAppender` in `logback.xml` is defined and used in the loggers.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on the issue:
https://github.com/apache/drill/pull/930
The proposal from @vrozov makes sense to me. As long as Lilith is not completely disabled, I'm fine with it.
---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/930
I have implemented the proposal of @vrozov to conditionally enable Lilith appender. I have used `ThresholdFilter` in Lilith appender, so it was not needed to change all places where it is used.
An example of how to enable Lilith was added to the Jira description.
@jinfengni, @vrozov could you please take a look?
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138100617
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,111 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
+ information regarding copyright ownership. The ASF licenses this file to
+ You under the Apache License, Version 2.0 (the "License"); you may not use
+ this file except in compliance with the License. You may obtain a copy of
+ the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<configuration>
+
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender name="SOCKET" class="de.huxhorn.lilith.logback.appender.ClassicMultiplexSocketAppender">
+ <Compressing>true</Compressing>
+ <ReconnectionDelay>10000</ReconnectionDelay>
+ <IncludeCallerData>true</IncludeCallerData>
+ <RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ </appender>
+ </then>
+ </if>
+
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <!-- encoders are assigned the type
+ ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <!--
+ <appender name="FILE" class="ch.qos.logback.core.rolling.RollingFileAppender">
+ <file>${log.path}</file>
+ <rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
+ <fileNamePattern>${log.path}.%i</fileNamePattern>
+ <minIndex>1</minIndex>
+ <maxIndex>10</maxIndex>
+ </rollingPolicy>
+
+ <triggeringPolicy class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
+ <maxFileSize>100MB</maxFileSize>
+ </triggeringPolicy>
+ <encoder>
+ <pattern>%date{ISO8601} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+ -->
+
+ <logger name="org.apache.drill" additivity="false">
+ <level value="debug"/>
--- End diff --
This logger contains only Lilith appender, and since we are using Lilith only for debugging separate tests, I think it would be better to left logging level here at `DEBUG`. In this case, everyone who uses Lilith won't need to change logback file. Console appender is used in the root logger and it has `ERROR` logging level.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r136835528
--- Diff: common/src/test/resources/logback.xml ---
@@ -16,17 +16,22 @@
<ReconnectionDelay>10000</ReconnectionDelay>
<IncludeCallerData>true</IncludeCallerData>
<RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ <!-- Disables Lilith ClassicMultiplexSocketAppender since its use causes hanging
--- End diff --
My guess is that using filter does not disable the appender and it is enabled but does not produce the output. There is still a question whether the appender tries to connect to the port and what is the behavior in case the connection can't be established (does it constantly retry to re-establish connection every 10 sec) or can be established but to HiveServer2 instead of Lilith server.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138408349
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,92 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
+ information regarding copyright ownership. The ASF licenses this file to
+ You under the Apache License, Version 2.0 (the "License"); you may not use
+ this file except in compliance with the License. You may obtain a copy of
+ the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<configuration>
+
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender name="SOCKET" class="de.huxhorn.lilith.logback.appender.ClassicMultiplexSocketAppender">
+ <Compressing>true</Compressing>
+ <ReconnectionDelay>10000</ReconnectionDelay>
+ <IncludeCallerData>true</IncludeCallerData>
+ <RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ </appender>
+ </then>
+ </if>
+
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <!-- encoders are assigned the type
+ ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <!--
--- End diff --
Is it necessary to have RollingFileAppender defined for unit test, especially that it is commented anyway?
---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the issue:
https://github.com/apache/drill/pull/930
LGTM. +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138620064
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,92 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
+ information regarding copyright ownership. The ASF licenses this file to
+ You under the Apache License, Version 2.0 (the "License"); you may not use
+ this file except in compliance with the License. You may obtain a copy of
+ the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<configuration>
+
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender name="SOCKET" class="de.huxhorn.lilith.logback.appender.ClassicMultiplexSocketAppender">
+ <Compressing>true</Compressing>
+ <ReconnectionDelay>10000</ReconnectionDelay>
+ <IncludeCallerData>true</IncludeCallerData>
+ <RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ </appender>
+ </then>
+ </if>
+
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <!-- encoders are assigned the type
+ ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <!--
--- End diff --
No, it is not necessary. It's hard to predict which appenders people use and since RollingFileAppender makes query execution slower, I decided to add it, but just in the comment.
Thanks for pointing this, I removed this comment with the appender.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138200345
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,111 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
+ information regarding copyright ownership. The ASF licenses this file to
+ You under the Apache License, Version 2.0 (the "License"); you may not use
+ this file except in compliance with the License. You may obtain a copy of
+ the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<configuration>
+
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender name="SOCKET" class="de.huxhorn.lilith.logback.appender.ClassicMultiplexSocketAppender">
+ <Compressing>true</Compressing>
+ <ReconnectionDelay>10000</ReconnectionDelay>
+ <IncludeCallerData>true</IncludeCallerData>
+ <RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ </appender>
+ </then>
+ </if>
+
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <!-- encoders are assigned the type
+ ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <!--
+ <appender name="FILE" class="ch.qos.logback.core.rolling.RollingFileAppender">
+ <file>${log.path}</file>
+ <rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
+ <fileNamePattern>${log.path}.%i</fileNamePattern>
+ <minIndex>1</minIndex>
+ <maxIndex>10</maxIndex>
+ </rollingPolicy>
+
+ <triggeringPolicy class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
+ <maxFileSize>100MB</maxFileSize>
+ </triggeringPolicy>
+ <encoder>
+ <pattern>%date{ISO8601} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+ -->
+
+ <logger name="org.apache.drill" additivity="false">
+ <level value="debug"/>
--- End diff --
OK.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138722693
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,92 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
+ information regarding copyright ownership. The ASF licenses this file to
+ You under the Apache License, Version 2.0 (the "License"); you may not use
+ this file except in compliance with the License. You may obtain a copy of
+ the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<configuration>
+
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender name="SOCKET" class="de.huxhorn.lilith.logback.appender.ClassicMultiplexSocketAppender">
+ <Compressing>true</Compressing>
+ <ReconnectionDelay>10000</ReconnectionDelay>
+ <IncludeCallerData>true</IncludeCallerData>
+ <RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ </appender>
+ </then>
+ </if>
+
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <!-- encoders are assigned the type
+ ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <!--
--- End diff --
+1 I think that developers are unlikely to use rolling file appender and can redirect console to a file if necessary.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r137252146
--- Diff: common/src/test/resources/logback.xml ---
@@ -16,17 +16,22 @@
<ReconnectionDelay>10000</ReconnectionDelay>
<IncludeCallerData>true</IncludeCallerData>
<RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ <!-- Disables Lilith ClassicMultiplexSocketAppender since its use causes hanging
--- End diff --
I agree with you that it does not disable Lilith. The connection is established with HiveServer2 instead of Lilith server and unit tests hang when `MultiplexSendBytesService` calls `sendBytes()` method.
I have updated changes, so now Lilith is disabled.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r137938315
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,111 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
+ information regarding copyright ownership. The ASF licenses this file to
+ You under the Apache License, Version 2.0 (the "License"); you may not use
+ this file except in compliance with the License. You may obtain a copy of
+ the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<configuration>
+
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender name="SOCKET" class="de.huxhorn.lilith.logback.appender.ClassicMultiplexSocketAppender">
+ <Compressing>true</Compressing>
+ <ReconnectionDelay>10000</ReconnectionDelay>
+ <IncludeCallerData>true</IncludeCallerData>
+ <RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ </appender>
+ </then>
+ </if>
+
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <!-- encoders are assigned the type
+ ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <!--
+ <appender name="FILE" class="ch.qos.logback.core.rolling.RollingFileAppender">
+ <file>${log.path}</file>
+ <rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
+ <fileNamePattern>${log.path}.%i</fileNamePattern>
+ <minIndex>1</minIndex>
+ <maxIndex>10</maxIndex>
+ </rollingPolicy>
+
+ <triggeringPolicy class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
+ <maxFileSize>100MB</maxFileSize>
+ </triggeringPolicy>
+ <encoder>
+ <pattern>%date{ISO8601} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+ -->
+
+ <logger name="org.apache.drill" additivity="false">
+ <level value="debug"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+<!-- <appender-ref ref="FILE" />-->
+ </logger>
+
+ <logger name="query.logger" additivity="false">
+ <level value="info"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+ </logger>
+
+ <!--
+ <logger name="io.netty" additivity="false">
+ <level value="debug"/>
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender-ref ref="SOCKET"/>
+ </then>
+ </if>
+ </logger>
+ -->
+
+ <logger name="org.apache.hadoop" additivity="false">
+ <level value="info"/>
--- End diff --
See comment below about MapR. It is not clear that we always want info-level logging from Hadoop. Seems this should be enabled only in tests that want that detail to avoid cluttering output with unwanted messages.
---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on the issue:
https://github.com/apache/drill/pull/930
Per this hive doc, seems it's possible to change hiveserver2 port #.
Can you try to change hivesever2# if the other one is hardcoded?
1. https://cwiki.apache.org/confluence/display/Hive/Setting+Up+HiveServer2
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Posted by jinfengni <gi...@git.apache.org>.
Github user jinfengni commented on the issue:
https://github.com/apache/drill/pull/930
We have run unit in our test clusters and I never hear that people complained that unit tests failed due to Lilith.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/930
@priteshm, as I have mentioned in the Jira [DRILL-5761](https://issues.apache.org/jira/browse/DRILL-5761), port in Lilith UI cannot be changed (https://github.com/huxi/lilith/issues/10).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138622881
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,111 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
+ information regarding copyright ownership. The ASF licenses this file to
+ You under the Apache License, Version 2.0 (the "License"); you may not use
+ this file except in compliance with the License. You may obtain a copy of
+ the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<configuration>
+
+ <if condition='property("drill.lilith.enable").equalsIgnoreCase("true")'>
+ <then>
+ <appender name="SOCKET" class="de.huxhorn.lilith.logback.appender.ClassicMultiplexSocketAppender">
+ <Compressing>true</Compressing>
+ <ReconnectionDelay>10000</ReconnectionDelay>
+ <IncludeCallerData>true</IncludeCallerData>
+ <RemoteHosts>${LILITH_HOSTNAME:-localhost}</RemoteHosts>
+ </appender>
+ </then>
+ </if>
+
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <!-- encoders are assigned the type
+ ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <!--
+ <appender name="FILE" class="ch.qos.logback.core.rolling.RollingFileAppender">
+ <file>${log.path}</file>
+ <rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
+ <fileNamePattern>${log.path}.%i</fileNamePattern>
+ <minIndex>1</minIndex>
+ <maxIndex>10</maxIndex>
+ </rollingPolicy>
+
+ <triggeringPolicy class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
+ <maxFileSize>100MB</maxFileSize>
+ </triggeringPolicy>
+ <encoder>
+ <pattern>%date{ISO8601} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+ -->
+
+ <logger name="org.apache.drill" additivity="false">
+ <level value="debug"/>
--- End diff --
Without comment with file appender yes, it can. Thanks, done.
---