You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by "cschneider (via GitHub)" <gi...@apache.org> on 2024/01/05 15:39:21 UTC
[PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
cschneider opened a new pull request, #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135
…ling
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "cschneider (via GitHub)" <gi...@apache.org>.
cschneider commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1880876252
I do not intend to offer the configuration to customers. I just use a configuration so we have a way to configure the maxQueueSize and maxDelay without doing a release.
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1878871161
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135) **Quality Gate passed**
Kudos, no new issues were introduced!
[0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&resolved=false&inNewCodePeriod=true)
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&resolved=false&inNewCodePeriod=true)
[96.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&metric=new_coverage&view=list)
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135)
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "cschneider (via GitHub)" <gi...@apache.org>.
cschneider commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1880552498
I have now implemented the new scheme of only starting to delay at queueSizeLimit. This should make sure we operate with full performance below that limit. Above the limit delays should not have any effect on throughput as a growing queue always signals that maximum throughput on publish side is reached.
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1882787069
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135) **Quality Gate passed**
Kudos, no new issues were introduced!
[0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&resolved=false&inNewCodePeriod=true)
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&resolved=false&inNewCodePeriod=true)
[91.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&metric=new_coverage&view=list)
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135)
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "tmaret (via GitHub)" <gi...@apache.org>.
tmaret commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1880772027
Assuming that customers will know what configuration value won't trigger rounding issues and/or select configurations that you assume are good values, seems a bit brittle and against the principle of validating user inputs. I suggest to pick any of those options to move forward :
(a) remove the configuration after validating the values ; or
(b) support the configuration range ; or
(c) limit the configuration range to what is effectively supported.
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1880541786
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135) **Quality Gate passed**
Kudos, no new issues were introduced!
[0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&resolved=false&inNewCodePeriod=true)
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&resolved=false&inNewCodePeriod=true)
[96.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&metric=new_coverage&view=list)
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135)
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "tmaret (via GitHub)" <gi...@apache.org>.
tmaret commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1880637632
Thank you, that is much better.
bq. I think the java arithmetic should be fine as the max delay is in milliseconds 20000 by default.
I would agree with you if the delay was not configurable. What happens if max delay is configured with a small number ?
bq. I don't think the delay increment has to correspond to the import latency if we start delaying later as you propose.
I think it should and be independent of the max queue size configuration. Best is to evaluate the dynamics with different configurations.
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1880546527
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135) **Quality Gate passed**
Kudos, no new issues were introduced!
[0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&resolved=false&inNewCodePeriod=true)
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&resolved=false&inNewCodePeriod=true)
[96.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&metric=new_coverage&view=list)
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135)
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "cschneider (via GitHub)" <gi...@apache.org>.
cschneider merged PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "cschneider (via GitHub)" <gi...@apache.org>.
cschneider commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1879579558
How about starting delay at queueSizeLimit and reaching max delay at queueSizeLimit *2.
So we could stay with the linear approach I have but simply start delaying later.
I think the java arithmetic should be fine as the max delay is in milliseconds 20000 by default.
I don't think the delay increment has to correspond to the import latency if we start delaying later as you propose.
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "cschneider (via GitHub)" <gi...@apache.org>.
cschneider commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1880877653
Of course this introduces the question if we should use a feature toggle.
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1881378531
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135) **Quality Gate passed**
Kudos, no new issues were introduced!
[0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&resolved=false&inNewCodePeriod=true)
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&resolved=false&inNewCodePeriod=true)
[90.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&metric=new_coverage&view=list)
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-distribution-journal&pullRequest=135)
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "tmaret (via GitHub)" <gi...@apache.org>.
tmaret commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1879238187
I suggest to rework the `getSleepTime` as follow:
1. Introduce a delay only when the queue size is > queueSizeLimit (0 delay otherwise)
2. Set the delay increment as the average (measured) import latency (should be something around 2ms)
3. Delay by (queueSize - queueSizeLimit) x delayIncrement / queueSizeLimit
4. Make sure that the delay computation is not flawed by Java int arithmetic (may need to leverage float / double)
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "cschneider (via GitHub)" <gi...@apache.org>.
cschneider commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1880731237
I think a max delay that is much smaller than 20.000 ms would not have much effect. So in practice I think it should not be a problem.
In worst case there might be some strange jumps in delay per increase in queueSize but for maxQueueSize we know the delay would be 0 and for maxQueueSize*2 we know the delay would be maxDelay. The steps in between do not matter that much as long as they are inside the range of [0,maxDelay], which they should be.
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12218 - Remove hard queue size limit. Introduce adaptive thrott… [sling-org-apache-sling-distribution-journal]
Posted by "tmaret (via GitHub)" <gi...@apache.org>.
tmaret commented on PR #135:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/135#issuecomment-1880927152
I think we should use a feature toggle and that we should go with (a) if there's no need for a customer configuration.
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org