vishesh92 commented on code in PR #8547:
URL: https://github.com/apache/cloudstack/pull/8547#discussion_r1466712708


##########
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java:
##########
@@ -121,7 +122,9 @@ private NicTO createNicTOFromNetworkAndOffering(NetworkVO 
networkVO, NetworkOffe
     public boolean hostConnect(long hostId, long poolId) throws 
StorageConflictException {
         StoragePool pool = (StoragePool) 
this.dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary);
         ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, 
pool);
-        cmd.setWait(60);
+        cmd.setWait(Wait.value() / 5);

Review Comment:
   Why do you think externalizing is the best choice?
   
   If we externalize this, we will be externalizing it for the default host 
listener only. Meaning the configuration parameter would be something like 
`command.modify.storage.pool.default.listener.timeout` since this won't be used 
by other listeners. And if we keep on externalizing in this way, eventually we 
will have a lot of configuration settings. We have around 550 Commands. Add 
listeners and providers on top, we will probably end up with more than a 
thousand new settings this way.
   
   I externalized ReadyCommand, because I could verify that not much is being 
done to process it.
   
   IMO, this is not the best solution as of now, but it's better than nothing. 
   I would prefer to actually have a better solution out of #8506 and use that 
instead.
   
   Let me know if this is okay for now. If it's not, I will just remove the 
wait for `ModifyStoragePoolCommand` for now.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to