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 02:21:48 UTC

[GitHub] [skywalking] wu-sheng opened a new pull request #8194: Revert IoTDB as storage option due to negative perf testing result.

wu-sheng opened a new pull request #8194:
URL: https://github.com/apache/skywalking/pull/8194


   <!--
       ⚠️ Please make sure to read this template first, pull requests that don't accord with this template
       maybe closed without notice.
       Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
       Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
   -->
   
   <!-- ==== 🐛 Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist 👇 ====
   ### Fix <bug description or the bug issue number or bug issue link>
   - [ ] Add a unit test to verify that the fix works.
   - [ ] Explain briefly why the bug exists and how to fix it.
        ==== 🐛 Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist 👆 ==== -->
   
   <!-- ==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👇 ====
   ### Improve the performance of <class or module or ...>
   - [ ] Add a benchmark for the improvement, refer to [the existing ones](https://github.com/apache/skywalking/blob/master/apm-commons/apm-datacarrier/src/test/java/org/apache/skywalking/apm/commons/datacarrier/LinkedArrayBenchmark.java)
   - [ ] The benchmark result.
   ```text
   <Paste the benchmark results here>
   ```
   - [ ] Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
        ==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👆 ==== -->
   
   <!-- ==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👇 ====
   ### <Feature description>
   - [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
   - [ ] Update the documentation to include this new feature.
   - [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
   - [ ] If it's UI related, attach the screenshots below.
        ==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👆 ==== -->
   
   - [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<issue number>.
   - [x] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/CHANGES.md).
   
   Revert #7766. I am so sorry @LIU-WEI-git @jixuan1989 I have to initialize this **revert** due to our recent performance testing result.
   
   Here are the graphs of SkyWalking OAP server so11y(self-obsevability) result.
   ![image](https://user-images.githubusercontent.com/5441976/143798527-5e0f38a7-379b-4d03-8f38-38b901ad57d2.png)
   ![image](https://user-images.githubusercontent.com/5441976/143798529-6eb48e02-38f8-481e-a148-59c77eec26a6.png)
   
   Before 9:45, the OAP includes the pull request of IoTDB storage implementation, you can see that, it provides much lower performance than it should be(after 9:45). The analysis capability drops from `30+k/s` to `< 10k/s`, which is hard to believe.
   
   Both of these testing are running in new deployed environment, most importantly, IoTDB storage option is even **NEVER ACTIVATED**, which could only provide one possibility, the packages(jars) we introduced have clear conflicts and performance impact to SkyWalking server. 
   This is the first time I see this happened, which surprised me too. But tests verified this repeated. I am submitting this pull request, and will ask @mrproliu to verify again according this branch.


-- 
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 pull request #8194: Revert IoTDB as storage option due to negative perf testing result.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #8194:
URL: https://github.com/apache/skywalking/pull/8194#issuecomment-981250365


   ![image](https://user-images.githubusercontent.com/5441976/143802387-5b4607f5-fd6b-4a53-83fe-784cb73be56f.png)
   ![image](https://user-images.githubusercontent.com/5441976/143802403-8238e99f-32ae-4804-baf7-721c6f338923.png)
   ![image](https://user-images.githubusercontent.com/5441976/143802416-d32da23b-07b2-4fc9-9cc6-59ebb4b57b0a.png)
   
   We have the 2nd time verification rechecked. Before 10:48, it was this PR's codebase testing, and after than we reset back to `master branch`(including IoTDB implementation for now).
   Same drop happened. 
   
   __
   
   ![image](https://user-images.githubusercontent.com/5441976/143802585-2ca26dfb-27db-48ad-b754-6c803821c3fe.png)
   
   According to this dependencies change, we are going to do one more attempting, which is, removing `logback-*`, even I think this has very little chance to affect this, due to we don't output logs many things during testing. But, this is only possible guess, others have to be rechecked by IoTDB team.


-- 
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 closed pull request #8194: Revert IoTDB as storage option due to negative perf testing result.

Posted by GitBox <gi...@apache.org>.
wu-sheng closed pull request #8194:
URL: https://github.com/apache/skywalking/pull/8194


   


-- 
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 pull request #8194: Revert IoTDB as storage option due to negative perf testing result.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #8194:
URL: https://github.com/apache/skywalking/pull/8194#issuecomment-981267038


   ![image](https://user-images.githubusercontent.com/5441976/143805003-e14654e8-aaf7-405e-bb67-c2e8fed248da.png)
   ![image](https://user-images.githubusercontent.com/5441976/143805020-a154544a-c551-43a0-85d2-d7973437051a.png)
   ![image](https://user-images.githubusercontent.com/5441976/143805026-e55a5aeb-41c1-484f-b470-911a6124ff8b.png)
   
   OK, after removing the logback, the perf seems back to normal. It seems log4j co-existing with logback causing huge performance impact. We are still checking.


-- 
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] jixuan1989 commented on pull request #8194: Revert IoTDB as storage option due to negative perf testing result.

Posted by GitBox <gi...@apache.org>.
jixuan1989 commented on pull request #8194:
URL: https://github.com/apache/skywalking/pull/8194#issuecomment-981254940


   We are keeping eye on this :D.


-- 
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 pull request #8194: Revert IoTDB as storage option due to negative perf testing result.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #8194:
URL: https://github.com/apache/skywalking/pull/8194#issuecomment-981270977


   @mrproliu If you could confirm this removing works, please submit a pull request to remove logback dependency.
   
   @kezhenxu94 Is there a chance we could add a blacklist in the dependency check script to avoid this case in the future?


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