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 2021/02/02 13:57:04 UTC
[GitHub] [cloudstack] ravening opened a new pull request #4641: Speed up SSVM startup time by postponing checks
ravening opened a new pull request #4641:
URL: https://github.com/apache/cloudstack/pull/4641
### Description
Currently SSVM takes long time to startup.
Reduce the startup time by moving volume/template
check to post connect TimerTask to speed up ssvm startup
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- 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: # -->
<!--- ********************************************************************************* -->
<!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ********************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [X] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [X] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [X] Minor
- [ ] Trivial
### 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] ravening closed pull request #4641: Speed up SSVM startup time by postponing checks
Posted by GitBox <gi...@apache.org>.
ravening closed pull request #4641:
URL: https://github.com/apache/cloudstack/pull/4641
--
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@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4641: Speed up SSVM startup time by postponing checks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4641:
URL: https://github.com/apache/cloudstack/pull/4641#discussion_r568636962
##########
File path: server/src/main/java/com/cloud/storage/download/DownloadListener.java
##########
@@ -284,18 +308,22 @@ public void processConnect(Host agent, StartupCommand cmd, boolean forRebalance)
_imageSrv.handleSysTemplateDownload(hostHyper, agent.getDataCenterId());
// update template_zone_ref for cross-zone templates
_imageSrv.associateCrosszoneTemplatesToZone(agent.getDataCenterId());
+ } else if (cmd instanceof StartupSecondaryStorageCommand) {
+ s_logger.debug("Scheduling postConnect task at " + STATUS_POLL_INTERVAL + " ms");
+ schedulePostConnectTask(agent, cmd, STATUS_POLL_INTERVAL);
}
- /* This can be removed
- else if ( cmd instanceof StartupStorageCommand) {
- StartupStorageCommand storage = (StartupStorageCommand)cmd;
- if( storage.getResourceType() == Storage.StorageResourceType.SECONDARY_STORAGE ||
- storage.getResourceType() == Storage.StorageResourceType.LOCAL_SECONDARY_STORAGE ) {
- downloadMonitor.addSystemVMTemplatesToHost(agent, storage.getTemplateInfo());
- downloadMonitor.handleTemplateSync(agent);
- downloadMonitor.handleVolumeSync(agent);
- }
- }*/
- else if (cmd instanceof StartupSecondaryStorageCommand) {
+ }
+
+ private void schedulePostConnectTask(Host agent, StartupCommand cmd, long delay) {
+ if (_postConnectTask != null)
+ _postConnectTask.cancel();
+
+ _postConnectTask = new PostConnectTask(this, agent, cmd);
+ _timer.schedule(_postConnectTask, delay);
+ }
+
+ private void postConnect(Host agent, StartupCommand cmd) throws ConnectionException {
+ if (cmd instanceof StartupSecondaryStorageCommand) {
Review comment:
this is only ever called with a `StartupSecondaryStorageCommand` so can we change the parameter type and skip the class check?
##########
File path: server/src/main/java/com/cloud/storage/download/DownloadListener.java
##########
@@ -284,18 +308,22 @@ public void processConnect(Host agent, StartupCommand cmd, boolean forRebalance)
_imageSrv.handleSysTemplateDownload(hostHyper, agent.getDataCenterId());
// update template_zone_ref for cross-zone templates
_imageSrv.associateCrosszoneTemplatesToZone(agent.getDataCenterId());
+ } else if (cmd instanceof StartupSecondaryStorageCommand) {
+ s_logger.debug("Scheduling postConnect task at " + STATUS_POLL_INTERVAL + " ms");
+ schedulePostConnectTask(agent, cmd, STATUS_POLL_INTERVAL);
}
- /* This can be removed
- else if ( cmd instanceof StartupStorageCommand) {
- StartupStorageCommand storage = (StartupStorageCommand)cmd;
- if( storage.getResourceType() == Storage.StorageResourceType.SECONDARY_STORAGE ||
- storage.getResourceType() == Storage.StorageResourceType.LOCAL_SECONDARY_STORAGE ) {
- downloadMonitor.addSystemVMTemplatesToHost(agent, storage.getTemplateInfo());
- downloadMonitor.handleTemplateSync(agent);
- downloadMonitor.handleVolumeSync(agent);
- }
- }*/
- else if (cmd instanceof StartupSecondaryStorageCommand) {
+ }
+
+ private void schedulePostConnectTask(Host agent, StartupCommand cmd, long delay) {
+ if (_postConnectTask != null)
+ _postConnectTask.cancel();
Review comment:
is this only ever working on one dowload job? it seems we are canceling the download check for someone else here.
----------------------------------------------------------------
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 a change in pull request #4641: Speed up SSVM startup time by postponing checks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4641:
URL: https://github.com/apache/cloudstack/pull/4641#discussion_r568637859
##########
File path: server/src/main/java/com/cloud/storage/download/DownloadListener.java
##########
@@ -284,18 +308,22 @@ public void processConnect(Host agent, StartupCommand cmd, boolean forRebalance)
_imageSrv.handleSysTemplateDownload(hostHyper, agent.getDataCenterId());
// update template_zone_ref for cross-zone templates
_imageSrv.associateCrosszoneTemplatesToZone(agent.getDataCenterId());
+ } else if (cmd instanceof StartupSecondaryStorageCommand) {
+ s_logger.debug("Scheduling postConnect task at " + STATUS_POLL_INTERVAL + " ms");
+ schedulePostConnectTask(agent, cmd, STATUS_POLL_INTERVAL);
}
- /* This can be removed
- else if ( cmd instanceof StartupStorageCommand) {
- StartupStorageCommand storage = (StartupStorageCommand)cmd;
- if( storage.getResourceType() == Storage.StorageResourceType.SECONDARY_STORAGE ||
- storage.getResourceType() == Storage.StorageResourceType.LOCAL_SECONDARY_STORAGE ) {
- downloadMonitor.addSystemVMTemplatesToHost(agent, storage.getTemplateInfo());
- downloadMonitor.handleTemplateSync(agent);
- downloadMonitor.handleVolumeSync(agent);
- }
- }*/
- else if (cmd instanceof StartupSecondaryStorageCommand) {
+ }
+
+ private void schedulePostConnectTask(Host agent, StartupCommand cmd, long delay) {
+ if (_postConnectTask != null)
+ _postConnectTask.cancel();
Review comment:
is this only ever working on one dowload job? it seems we are canceling the download check for someone else here.
----------------------------------------------------------------
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 #4641: Speed up SSVM startup time by postponing checks
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4641:
URL: https://github.com/apache/cloudstack/pull/4641#issuecomment-914909411
Ping @ravening
--
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@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4641: Speed up SSVM startup time by postponing checks
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4641:
URL: https://github.com/apache/cloudstack/pull/4641#issuecomment-919099148
> Pl discuss on dev@, in my opinion this should be closed as postponing check isn't something we would want. The tradeoff of speedup vs check needs to be discussed.
@ravening
maybe I miss something, I think this is good (it has been used for many years as far as I know).
anyway, it is not a major bug fix.
--
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@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4641: Speed up SSVM startup time by postponing checks
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4641:
URL: https://github.com/apache/cloudstack/pull/4641#issuecomment-919115487
> @weizhouapache I'm just being risk-averse :) I see that checks are only postponed - but I worry if not doing checks at the time of initial connection causes any failures? For example, you allow the agent to connect and it starts doing some storage related operation, and later the post-connect thread/check runs and finds out we shouldn't have connected, or it fails but since the agent is connected, agent continues to process new commands? (a case can be the template/volume scanning logic times out, but the connect is still connected so agent gets some new operation say re-download template from a URL which is wrong/404 etc and it ends up overwriting existing files?)
@rhtyd these are very good questions.
@ravening are you ok to close this pr ? the issue this pr aims to fix is, SSVM will be Up only if all imags/volumes are checked which I think it is not a major 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4641: Speed up SSVM startup time by postponing checks
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4641:
URL: https://github.com/apache/cloudstack/pull/4641#issuecomment-919102406
@weizhouapache I'm just being risk-averse :) I see that checks are only postponed - but I worry if not doing checks at the time of initial connection causes any failures? For example, you allow the agent to connect and it starts doing some storage related operation, and later the post-connect thread/check runs and finds out we shouldn't have connected, or it fails but since the agent is connected, agent continues to process new commands? (a case can be the template/volume scanning logic times out, but the connect is still connected so agent gets some new operation say re-download template from a URL which is wrong/404 etc and it ends up overwriting existing files?)
--
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@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4641: Speed up SSVM startup time by postponing checks
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4641:
URL: https://github.com/apache/cloudstack/pull/4641#issuecomment-918844695
Pl discuss on dev@, in my opinion this should be closed as postponing check isn't something we would want. The tradeoff of speedup vs check needs to be discussed.
--
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@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on pull request #4641: Speed up SSVM startup time by postponing checks
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4641:
URL: https://github.com/apache/cloudstack/pull/4641#issuecomment-919120636
closing it after discussion
--
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@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4641: Speed up SSVM startup time by postponing checks
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4641:
URL: https://github.com/apache/cloudstack/pull/4641#discussion_r568636962
##########
File path: server/src/main/java/com/cloud/storage/download/DownloadListener.java
##########
@@ -284,18 +308,22 @@ public void processConnect(Host agent, StartupCommand cmd, boolean forRebalance)
_imageSrv.handleSysTemplateDownload(hostHyper, agent.getDataCenterId());
// update template_zone_ref for cross-zone templates
_imageSrv.associateCrosszoneTemplatesToZone(agent.getDataCenterId());
+ } else if (cmd instanceof StartupSecondaryStorageCommand) {
+ s_logger.debug("Scheduling postConnect task at " + STATUS_POLL_INTERVAL + " ms");
+ schedulePostConnectTask(agent, cmd, STATUS_POLL_INTERVAL);
}
- /* This can be removed
- else if ( cmd instanceof StartupStorageCommand) {
- StartupStorageCommand storage = (StartupStorageCommand)cmd;
- if( storage.getResourceType() == Storage.StorageResourceType.SECONDARY_STORAGE ||
- storage.getResourceType() == Storage.StorageResourceType.LOCAL_SECONDARY_STORAGE ) {
- downloadMonitor.addSystemVMTemplatesToHost(agent, storage.getTemplateInfo());
- downloadMonitor.handleTemplateSync(agent);
- downloadMonitor.handleVolumeSync(agent);
- }
- }*/
- else if (cmd instanceof StartupSecondaryStorageCommand) {
+ }
+
+ private void schedulePostConnectTask(Host agent, StartupCommand cmd, long delay) {
+ if (_postConnectTask != null)
+ _postConnectTask.cancel();
+
+ _postConnectTask = new PostConnectTask(this, agent, cmd);
+ _timer.schedule(_postConnectTask, delay);
+ }
+
+ private void postConnect(Host agent, StartupCommand cmd) throws ConnectionException {
+ if (cmd instanceof StartupSecondaryStorageCommand) {
Review comment:
this is only ever called with a `StartupSecondaryStorageCommand` so can we change the parameter type and skip the class check?
----------------------------------------------------------------
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] ravening commented on pull request #4641: Speed up SSVM startup time by postponing checks
Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4641:
URL: https://github.com/apache/cloudstack/pull/4641#issuecomment-915039145
@weizhouapache you have any feedback on this pr?
--
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@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org