You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/11/06 08:56:33 UTC
[GitHub] [cloudstack] DaanHoogland opened a new pull request #4451: loop optimisation in bash
DaanHoogland opened a new pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451
## Description
<!--- Describe your changes in detail -->
A small optimisation as suggested by the user in #4449 . I'm not convinced this is actually reducing the memory usage as they suggest. the code itself seems rathrer risk-free. let's discuss.
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
Fixes: #4449
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [x] Cleanup (Code refactoring and cleanup, that may add test cases)
## Screenshots (if appropriate):
## How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-744227256
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2489
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-744232768
@blueorangutan test
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-732019871
@davidjumani I like the idea, do we have a good logical splitting of the file? the problem states that many firewall rules cause the issue , so splitting over functional parts wouldn't solve this.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] wido commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
wido commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-722979943
I'm not sure if this fixes it. Not against changing it, but I don't know if this is the root-cause.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] davidjumani commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-732134868
@DaanHoogland Assuming that the rules are in multiple lines, loading all the lines in memory and processing them would kill the VR. Instead splitting the file into smaller ones and reading them could solve the memory issue
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-744205568
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] davidjumani commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-732013237
@rhtyd @DaanHoogland Would splitting the file and processing each part sequentially work ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-770739797
> I don't care for this code @rhtyd . I don't know but i don't think it is an improvement/optimisation
same to me.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-722961525
@GabrielBrascher @weizhouapache @rhtyd @wido let's discuss this. I've seen issues in large isolated nets on startup but I doubt this is going to be a big help. thoughts?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-770728689
I don't care for this code @rhtyd . I don't know but i don't think it is an improvement/optimisation
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-770696133
@DaanHoogland @weizhouapache @wido should we merge this for 4.15.1.0 (targetting Q2)?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-744613586
<b>Trillian test result (tid-3346)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36386 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4451-t3346-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 85 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_06_download_detached_volume | `Failure` | 312.64 | test_volumes.py
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-744418006
> @DaanHoogland @rhtyd
> I suggest not to merge this pr into 4.15.0.0
I agree, and would say let's not merge at all until we have knowledge if this is making a difference at all.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd merged pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-781318965
Considering all I could find, the pipe based thing would be better compared to redirection as it would avoid memory exhaustion; the pipe/buffer synchronisation (https://stackoverflow.com/questions/9553628/piping-and-redirection). Let's merge this in 4.15 and test over next few weeks, we can revisit and revert if necessary.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-744233293
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-744204990
@blueorangutan package
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-744256558
@DaanHoogland @rhtyd
I suggest not to merge this pr into 4.15.0.0
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-744268766
Yes @weizhouapache, it's marked against 4.15.1.0; (I'm) just kicking tests on PRs.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4451: loop optimisation in bash
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4451:
URL: https://github.com/apache/cloudstack/pull/4451#issuecomment-732148370
Again @davidjumani I like the idea. What is the criterium? (no of lines, sections some combination of criterea) can we be sure that lines are self contained and do not require context or order of executing and can we guarantee that?
----------------------------------------------------------------
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:
users@infra.apache.org