Whirr
  1. Whirr
  2. WHIRR-220 Support local tarball upload
  3. WHIRR-280

Create a blob cache that could be used for storing local files

    Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.5.0
    • Component/s: core
    • Labels:
      None

      Description

      Create a new class (BlobCache) that can be used as a temporary blob store for local files in the cloud. This class should be able to create jclouds Statements that could download files on the cluster nodes as needed.

      1. WHIRR-280.patch
        8 kB
        Andrei Savu
      2. WHIRR-280.patch
        9 kB
        Andrei Savu
      3. WHIRR-280.patch
        19 kB
        Andrei Savu

        Issue Links

          Activity

          Hide
          Andrei Savu added a comment -

          Basic functionality and an incomplete test (svn git mirror is out of sync and I'm having some issues while integrating multiple patches).

          Show
          Andrei Savu added a comment - Basic functionality and an incomplete test (svn git mirror is out of sync and I'm having some issues while integrating multiple patches).
          Hide
          Andrei Savu added a comment -

          Fixed BlobCache integration test. I had to make a commit to remove some empty folders to get the git mirror back in sync.

          Show
          Andrei Savu added a comment - Fixed BlobCache integration test. I had to make a commit to remove some empty folders to get the git mirror back in sync.
          Hide
          Tom White added a comment -

          I just removed some more empty directories (commit 1091845).

          Show
          Tom White added a comment - I just removed some more empty directories (commit 1091845).
          Hide
          Adrian Cole (Inactive) added a comment -

          can't quite apply this patch as WHIRR-279 in progress.

          Some notes:
          I think the checkNull singleton pattern is a bit distracting. I prefer java enum singletons but not hard set on it.

          Also, it might be cleaner to use the InputStreamMap or BlobMap in jclouds rather that this object. It is a view that supports containsKey() clear() etc, and could probably eliminate most of this code. Ideally, we could create a small function to make a concurrent map out of this, emulating putIfAbsent() with something like this

          if (!map.containsKey(key))
          return map.put(key, value);
          else
          return map.get(key);

          ex. ConcurrentMap<String, Blob> cache = makeConcurrentMap(deleteContainerOnShutdown(context.getBlobMap(createTempContainer())));

          cache.clear()// deletes all blobs
          cache.putIfAbsent("key", blobFrom(uri));

          you'd think that this would be very inefficient, as maps need to return the old value. However, since there is no old value (new container), the performance should be fine.

          my 2p

          Show
          Adrian Cole (Inactive) added a comment - can't quite apply this patch as WHIRR-279 in progress. Some notes: I think the checkNull singleton pattern is a bit distracting. I prefer java enum singletons but not hard set on it. Also, it might be cleaner to use the InputStreamMap or BlobMap in jclouds rather that this object. It is a view that supports containsKey() clear() etc, and could probably eliminate most of this code. Ideally, we could create a small function to make a concurrent map out of this, emulating putIfAbsent() with something like this if (!map.containsKey(key)) return map.put(key, value); else return map.get(key); ex. ConcurrentMap<String, Blob> cache = makeConcurrentMap(deleteContainerOnShutdown(context.getBlobMap(createTempContainer()))); cache.clear()// deletes all blobs cache.putIfAbsent("key", blobFrom(uri)); you'd think that this would be very inefficient, as maps need to return the old value. However, since there is no old value (new container), the performance should be fine. my 2p
          Hide
          Andrei Savu added a comment -

          Adrian, thanks for feedback. Sounds like a good approach and I will give it a try tomorrow.

          Show
          Andrei Savu added a comment - Adrian, thanks for feedback. Sounds like a good approach and I will give it a try tomorrow.
          Hide
          Andrei Savu added a comment -

          Updated patch to do automatic location selection and I've added to BlobCache a method that can be used to create jclouds download Statements.

          Now I'm going to provide a new patch for WHIRR-220 based on the sub-tasks.

          Show
          Andrei Savu added a comment - Updated patch to do automatic location selection and I've added to BlobCache a method that can be used to create jclouds download Statements. Now I'm going to provide a new patch for WHIRR-220 based on the sub-tasks.
          Hide
          Tom White added a comment -

          This looks good to me. Automatic location selection is very cool. BTW is it possible to use BlobMap or InputStreamMap per Adrian's feedback?

          Very minor comment: containsAny can be implemented in one line as !Sets.newHashSet(set1).retainAll(set2).isEmpty() (set intersection).

          Show
          Tom White added a comment - This looks good to me. Automatic location selection is very cool. BTW is it possible to use BlobMap or InputStreamMap per Adrian's feedback? Very minor comment: containsAny can be implemented in one line as !Sets.newHashSet(set1).retainAll(set2).isEmpty() (set intersection).
          Hide
          Adrian Cole (Inactive) added a comment -

          I'm ok with this not using blobMap, just that I think it will be more testable when we do. perhaps we can open this idea up as a separate enhancement request and I can do this afterwards?

          Show
          Adrian Cole (Inactive) added a comment - I'm ok with this not using blobMap, just that I think it will be more testable when we do. perhaps we can open this idea up as a separate enhancement request and I can do this afterwards?
          Hide
          Andrei Savu added a comment -

          Adrian let's do that in a separate issue.

          Show
          Andrei Savu added a comment - Adrian let's do that in a separate issue.
          Hide
          Andrei Savu added a comment -

          I've just committed this. Thanks guys for review!

          Show
          Andrei Savu added a comment - I've just committed this. Thanks guys for review!

            People

            • Assignee:
              Andrei Savu
              Reporter:
              Andrei Savu
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development