You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/11/29 07:25:06 UTC
[GitHub] [skywalking] mrproliu opened a new pull request #8202: Remove `logback` dependencies in IoTDB plugin
mrproliu opened a new pull request #8202:
URL: https://github.com/apache/skywalking/pull/8202
### Improve the performance of ALS message receiver, follow https://github.com/apache/skywalking/pull/8194#issuecomment-981267038
- [x] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/CHANGES.md).
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #8202: Remove `logback` dependencies in IoTDB plugin
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #8202:
URL: https://github.com/apache/skywalking/pull/8202#discussion_r758161656
##########
File path: oap-server/server-storage-plugin/storage-iotdb-plugin/pom.xml
##########
@@ -49,6 +49,16 @@
<groupId>net.jpountz.lz4</groupId>
<artifactId>lz4</artifactId>
</exclusion>
+ <!-- logback has runtime negative impact about performance, -->
Review comment:
> I added this, not @mrproliu
Don't take it personal, I was just commenting in the perspective of logback's community, no matter who, without benchmark test result / evidence, saying others have pool performance is not cool, I still remember someone said SkyWalking is pool performant in Twitter, but they were not aware of that they misused SkyWalking, I felt the same uncomfortable that time.
> I am thinking the performance impact is real, but we don't know why.
The performance impact is real, but we can only say it exists when logback + log4j2 + slf4j are all in the classpath, if we really want to put this sentence, we at least should test another case where we remove log4j2 and keep logback + slf4j.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a change in pull request #8202: Remove `logback` dependencies in IoTDB plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8202:
URL: https://github.com/apache/skywalking/pull/8202#discussion_r758113580
##########
File path: oap-server/server-storage-plugin/storage-iotdb-plugin/pom.xml
##########
@@ -49,6 +49,14 @@
<groupId>net.jpountz.lz4</groupId>
<artifactId>lz4</artifactId>
</exclusion>
+ <exclusion>
Review comment:
```suggestion
<!-- logback has runtime negative impact about performance, -->
<!-- Disable this, and use slf4j-sl4j instead of. -->
<exclusion>
```
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a change in pull request #8202: Remove `logback` dependencies in IoTDB plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8202:
URL: https://github.com/apache/skywalking/pull/8202#discussion_r758139004
##########
File path: oap-server/server-storage-plugin/storage-iotdb-plugin/pom.xml
##########
@@ -49,6 +49,16 @@
<groupId>net.jpountz.lz4</groupId>
<artifactId>lz4</artifactId>
</exclusion>
+ <!-- logback has runtime negative impact about performance, -->
Review comment:
I added this, not @mrproliu
I am thinking the performance impact is real, but we don't know why.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] mrproliu commented on a change in pull request #8202: Remove `logback` dependencies in IoTDB plugin
Posted by GitBox <gi...@apache.org>.
mrproliu commented on a change in pull request #8202:
URL: https://github.com/apache/skywalking/pull/8202#discussion_r758109040
##########
File path: dist-material/release-docs/LICENSE
##########
@@ -398,10 +398,6 @@ EPL licenses
The following components are provided under the EPL License. See project link for details.
The text of each license is also included at licenses/LICENSE-[project].txt.
Review comment:
been removed, forget to update this before.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #8202: Remove `logback` dependencies in IoTDB plugin
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #8202:
URL: https://github.com/apache/skywalking/pull/8202#discussion_r758137983
##########
File path: oap-server/server-storage-plugin/storage-iotdb-plugin/pom.xml
##########
@@ -49,6 +49,16 @@
<groupId>net.jpountz.lz4</groupId>
<artifactId>lz4</artifactId>
</exclusion>
+ <!-- logback has runtime negative impact about performance, -->
Review comment:
IMHO this is a kind of arbitrary decision and we didn't even have a solid conclusion about the reason of https://github.com/apache/skywalking/pull/8194 , let's remove this kind of sentence, to me the only reason to exclude this is that we already have slf4j implementation (log4j2) in the classpath
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a change in pull request #8202: Remove `logback` dependencies in IoTDB plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8202:
URL: https://github.com/apache/skywalking/pull/8202#discussion_r758166305
##########
File path: oap-server/server-storage-plugin/storage-iotdb-plugin/pom.xml
##########
@@ -49,6 +49,18 @@
<groupId>net.jpountz.lz4</groupId>
<artifactId>lz4</artifactId>
</exclusion>
+ <!-- When logback + log4j2 + slf4j co-existed in OAP server classpath at the runtime, -->
+ <!-- SkyWalking so11y reported a runtime negative impact about performance. -->
+ <!-- Disable this, and use slf4j-log4j instead of. -->
+ <!-- Ref original discussion, https://github.com/apache/skywalking/pull/8194 -->
Review comment:
@kezhenxu94 Please recheck.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng merged pull request #8202: Remove `logback` dependencies in IoTDB plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #8202:
URL: https://github.com/apache/skywalking/pull/8202
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] mrproliu commented on a change in pull request #8202: Remove `logback` dependencies in IoTDB plugin
Posted by GitBox <gi...@apache.org>.
mrproliu commented on a change in pull request #8202:
URL: https://github.com/apache/skywalking/pull/8202#discussion_r758156649
##########
File path: dist-material/release-docs/LICENSE
##########
@@ -391,17 +391,6 @@ CC0-1.0 licenses
HdrHistogram 2.1.12: https://github.com/HdrHistogram/HdrHistogram , CC0-1.0 and BSD 2-Clause
reactive-streams 1.0.2: https://github.com/reactive-streams/reactive-streams-jvm , CC0-1.0
-========================================================================
-EPL licenses
-========================================================================
-
-The following components are provided under the EPL License. See project link for details.
-The text of each license is also included at licenses/LICENSE-[project].txt.
-
- logback 1.2.3: https://github.com/qos-ch/logback: EPL 1.0
Review comment:
Ok, I see the web app has used `logback-classic` and `logback-core`, I will added back.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a change in pull request #8202: Remove `logback` dependencies in IoTDB plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8202:
URL: https://github.com/apache/skywalking/pull/8202#discussion_r758107647
##########
File path: dist-material/release-docs/LICENSE
##########
@@ -398,10 +398,6 @@ EPL licenses
The following components are provided under the EPL License. See project link for details.
The text of each license is also included at licenses/LICENSE-[project].txt.
Review comment:
You should remove these licenses.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a change in pull request #8202: Remove `logback` dependencies in IoTDB plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8202:
URL: https://github.com/apache/skywalking/pull/8202#discussion_r758163239
##########
File path: oap-server/server-storage-plugin/storage-iotdb-plugin/pom.xml
##########
@@ -49,6 +49,16 @@
<groupId>net.jpountz.lz4</groupId>
<artifactId>lz4</artifactId>
</exclusion>
+ <!-- logback has runtime negative impact about performance, -->
Review comment:
I can understand the statement to co-existing to make more accurate. And ref the PR with screenshots as prove.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #8202: Remove `logback` dependencies in IoTDB plugin
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #8202:
URL: https://github.com/apache/skywalking/pull/8202#discussion_r758134220
##########
File path: dist-material/release-docs/LICENSE
##########
@@ -391,17 +391,6 @@ CC0-1.0 licenses
HdrHistogram 2.1.12: https://github.com/HdrHistogram/HdrHistogram , CC0-1.0 and BSD 2-Clause
reactive-streams 1.0.2: https://github.com/reactive-streams/reactive-streams-jvm , CC0-1.0
-========================================================================
-EPL licenses
-========================================================================
-
-The following components are provided under the EPL License. See project link for details.
-The text of each license is also included at licenses/LICENSE-[project].txt.
-
- logback 1.2.3: https://github.com/qos-ch/logback: EPL 1.0
Review comment:
I'm afraid this needs to be reserved because we still use logback in web app and it's in our distributed tar.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org