https://issues.apache.org/jira/browse/JCLOUDS-512

TL;DR: A proper image cache for the compute abstraction.

Apologies for quite long PR description, but I've been thinking about this for 
a long time and approached the problem in several ways. This is the less 
intrusive fix I've come up with after several painful refactors. The core 
changes are not obvious in the PR diff, so I'll try to summarise them.

**Background:**
Images from a provider are exposed to the user as a `Set`. Internally, that set 
is accessed through a memoized `Supplier` that acts as a cache, and. jclouds 
uses it to improve response times when talking to providers where getting the 
image list is an expensive operation.

This cached set, as it is *immutable* gives little control over its lifecycle, 
and this presents issues with the `ImageExtension`. Users can use it to create 
and delete images in the provider, but those changes are not reflected in the 
cache (given its nature), and inconsistent behaviors start to appear in 
subsequent uses of the `TemplateBuilder`.

**Proposed fix:**
Stop using a cached set and use a proper `Cache<String, Image>` structure. To 
avoid having to refactor all the providers, as the injection of the cached 
`Supplier<Set<Image>>` is a common practice, the cache  will also implement 
`Supplier<Set<Image>>`.

**Implementation details and considerations:**
* The `ComputeService` returns a `Set<Image>`, and it is convenient to keep 
that signature; we don't want to couple it to a `Cache` and the fact that 
images are cached by jclouds.
* We want to keep the "old" `Supplier<Set<Image>>` to allow users to call it to 
(lazily) load the images in the provider, but we want to "move" the caching 
thing out of it.
* The `ImageCacheSupplier` (the `LoadingCache` implementation) uses the 
`GetImagesStrategy` to load individual images, and is initialised with the 
values of the `Supplier<Set<Image>>`. As the supplied set is lazy loaded, we 
don't want to call it when building the cache: it has to be initialised only 
when the supplier "loads live values". This has been resolved by generating an 
event when the supplier loads new values, and by subscribing the image cache to 
that event. This way the cache is populated with the images exactly when new 
values are fetched.
* A default implementation of the `ImageExtension` has been added to update the 
cache every time images are aded or deleted.
* The `TemplateBuilder` now provides a method to bypass and reload the cache, 
as this has been asked several times:
```java
Template template = compute.templateBuilder().forceCacheReload(). ... .build();
```

I still have to run live tests in several providers before merging, but I'd 
like to get some early feedback:

@demobox @abayer @ccustine Your comments would be highly appreciated
@felfert Could you try this branch and see if it fixes the caching issues you 
were experiencing with Jenkins?
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds/pull/885

-- Commit Summary --

  * JCLOUDS-512: Implement the ImageCache

-- File Changes --

    M 
apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/config/CloudStackComputeServiceContextModule.java
 (11)
    M 
apis/ec2/src/main/java/org/jclouds/ec2/compute/config/EC2ComputeServiceContextModule.java
 (19)
    M 
apis/ec2/src/main/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImpl.java
 (8)
    M 
apis/ec2/src/test/java/org/jclouds/ec2/compute/EC2TemplateBuilderTest.java (9)
    M 
apis/ec2/src/test/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImplTest.java
 (10)
    M 
apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/config/NovaComputeServiceContextModule.java
 (11)
    M 
compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java
 (51)
    M compute/src/main/java/org/jclouds/compute/domain/TemplateBuilder.java (16)
    M compute/src/main/java/org/jclouds/compute/domain/TemplateBuilderSpec.java 
(25)
    M 
compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java
 (81)
    A 
compute/src/main/java/org/jclouds/compute/extensions/internal/DelegatingImageExtension.java
 (74)
    M 
compute/src/main/java/org/jclouds/compute/internal/BaseComputeService.java (9)
    M 
compute/src/main/java/org/jclouds/compute/stub/config/StubComputeServiceContextModule.java
 (9)
    M 
compute/src/main/java/org/jclouds/compute/suppliers/ImageCacheSupplier.java 
(182)
    M 
compute/src/test/java/org/jclouds/compute/domain/TemplateBuilderSpecTest.java 
(51)
    M 
compute/src/test/java/org/jclouds/compute/domain/internal/TemplateBuilderImplTest.java
 (9)
    M 
compute/src/test/java/org/jclouds/compute/extensions/internal/BaseImageExtensionLiveTest.java
 (40)
    M 
compute/src/test/java/org/jclouds/compute/suppliers/ImageCacheSupplierTest.java 
(80)
    M 
core/src/main/java/org/jclouds/rest/suppliers/MemoizedRetryOnTimeOutButNotOnAuthorizationExceptionSupplier.java
 (45)
    M 
core/src/test/java/org/jclouds/rest/suppliers/MemoizedRetryOnTimeOutButNotOnAuthorizationExceptionSupplierTest.java
 (79)
    M 
providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2TemplateBuilderImpl.java
 (8)
    M 
providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/config/AWSEC2ComputeServiceContextModule.java
 (21)
    M 
providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/config/GoogleComputeEngineServiceContextModule.java
 (42)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/885.patch
https://github.com/jclouds/jclouds/pull/885.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/885

Reply via email to