[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8988?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15033662#comment-15033662
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8988:
--------------------------------------------

Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/943#issuecomment-160966699
  
    @DaanHoogland, 
    I checked the logs you sent me.
    
    The VMs were marked as destroyed, but it seems that they have not been 
“destroyed” or removed/expunged yet. I looked at the code, and the only way 
that they are removed from the response of the list VMs methods is after the 
expunge thread execution that fills out the “removed” field in the database.
     
    I also looked at the code of the integration tests, my perl is a little 
rusty, but I noticed that the code waits a few cycles (2) of the expunge delay 
to execute; therefore, there is no way to guarantee that the expunge thread has 
already been executed and the VM has passed the expunge delay and has been 
removed.
    
    If I recall properly, there are mainly three (3) variables in play, the 
time that the VM was destroyed, the expunge delay per se and the expunge 
interval (the interval of the expunge thread execution). 
    
    So, if the expunge thread runs, but the VM has been destroyed too recently 
and has not passed the expunge delay, it will not be marked as destroyed. That 
is what seems to have happened there. I know some people may come and say, “the 
test worked a lot of time”. And yes it can work, but it depends if you are luck 
or not. I personally do not like tests that may present this kind of behavior.  
Moreover, the expunge interval depends on the time that the MS has been started.
    
    I will illustrate it with an example that we have seen happening here.
    Giving that our expunge interval is 24 hours, and our expunge delay is also 
24 hours. Suppose the MS server was started and got up and running at some day 
at 23:59 and that the first time the expunge thread runs is 00:00. If we are 
unlucky and we destroy the VM at 00:01, next day (second run of the expunge 
thread) when the thread runs at 00:00, the VM will not be removed and will 
continue appearing, since the expunge delay that cotrols the VMs removal is 24 
hours and the VM has been destroyed for 23:59 (almost there, but not yet). 
Therefore, the VM will only be removed in the third execution of the expunge 
thread.
    
    Having said that, I have the following questions, what do we want with that 
test? We want to test the expunge thread? Or just test If the destroyed VM is 
not listed? If we want the second, why don’t we force the expunging (using 
expungeVirtualMachine command) instead of waiting the expunge thread?
    
    If the idea is to let the test as it is, to avoid the problem I have just 
described, we could just change a "bit" of the file test_vm_life_cycle the 
multiplier, in line 632 from “4” to “6” . That change would guarantee to wait 
till the third execution of the expunge thread, and avoid cases as the one 
described.


> CLOUDSTACK-8988
> ---------------
>
>                 Key: CLOUDSTACK-8988
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8988
>             Project: CloudStack
>          Issue Type: Improvement
>      Security Level: Public(Anyone can view this level - this is the 
> default.) 
>          Components: Management Server, Projects
>    Affects Versions: 4.7.0
>         Environment: Windows 10; Eclipse;
>            Reporter: Rodrigo Pedro Marques
>            Priority: Minor
>              Labels: easyfix, github-import
>             Fix For: 4.7.0
>
>   Original Estimate: 0h
>  Remaining Estimate: 0h
>
> Removal of cloud-plugin-storage-allocator-random project that was unused.
> File modified: /cloud-server/test/async-job-component.xml. Removed some 
> unused adapters.The reason for this is explained as follows.
> The adapter configuration is the following:
> <adapters key="com.cloud.agent.manager.allocator.StorageAllocator">
> <adapter name="Storage"
> class="com.cloud.agent.manager.allocator.impl.FirstFitStorageAllocator">
> <param name="storage.overprovisioning.factor">2</param>
> </adapter>
> <adapter name="
> class="com.cloud.agent.manager.allocator.impl.RandomStoragePoolAllocator">
> <param name="storage.overprovisioning.factor">2</param>
> </adapter>
> </adapters>
> • class="com.cloud.agent.manager.allocator.impl.FirstFitStorageAllocator"
> The class "com.cloud.agent.manager.allocator.impl.FirstFitStorageAllocator" 
> does not exist. The only reference for it is found in the following file:
> - /cloud-server/test/async-job-component.xml
> Therefore, we can conclude that there is no need for this line at that file.
> • class="com.cloud.agent.manager.allocator.impl.RandomStoragePoolAllocator"
> Additionally, the class RandomStoragePoolAllocator.java is never used. The 
> only reference is found in the following file:
> /cloud-server/test/async-job-component.xml
> We found a project called “cloud-plugin-storage-allocator-random”. This 
> project has only one package that contains only one class, which is the 
> RandomStoragePoolAllocator.java. Despite the names that are the same, the 
> class in “cloud-plugin-storage-allocator-random” project and the class 
> referenced in - /cloud-server/test/async-job-component.xml have different 
> packages. Therefore, we removed that configuration from 
> async-job-component.xml and the project that contains only the 
> RandomStoragePoolAllocator class that is never used.
> Consequently, we had to remove the following lines from the 
> /cloud-client-ui/pom.xml:
> <dependency>
> <groupId>org.apache.cloudstack</groupId>
> <artifactId>cloud-plugin-storage-allocator-random</artifactId>
> <version>${project.version}</version>
> </dependency>
> Those changes leave us with an adapter configuration empty with the following 
> key:
> • key="com.cloud.agent.manager.allocator.StorageAllocator"
> Therefore, we removed it.
> Furthermore, after we removed that configuration we noticed that there is no 
> such class StorageAllocator.java. However, it appears that exists test for 
> it, like the following classes:
> StorageAllocatorTestConfiguration.java
> StorageAllocatorTest.java. We are not sure if these classes are tests for the 
> class StorageAllocator.java and for the possible configuration we have just 
> removed. If they are, we can remove both classes. 
> We also removed the following configuration from /cloudstack-plugins/pom.xml:
> <module>storage-allocators/random</module>



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to