Python API: Allow to cache object metadata and location #2009

AymericDu · 2020-03-25T21:08:18Z

SUMMARY

Allow to cache object metadata

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

Python API

SDS VERSION

openio 4.8.3.dev5

codecov-io · 2020-03-26T10:19:51Z

Codecov Report

Merging #2009 into 4.x will decrease coverage by 0.15%.
The diff coverage is 95.84%.

@@            Coverage Diff             @@
##              4.x    #2009      +/-   ##
==========================================
- Coverage   80.84%   80.69%   -0.14%     
==========================================
  Files         359      375      +16     
  Lines       69015    70709    +1694     
==========================================
+ Hits        55788    57053    +1265     
- Misses      13227    13534     +307     
- Partials        0      122     +122

Impacted Files	Coverage Δ
oio/common/storage_functions.py	`90.07% <50.00%> (-1.02%)`	⬇️
oio/common/cache.py	`93.75% <93.75%> (ø)`
oio/container/client.py	`84.45% <95.00%> (+0.53%)`	⬆️
tests/functional/api/test_objectstorage.py	`97.46% <98.43%> (+0.02%)`	⬆️
oio/api/object_storage.py	`80.82% <100.00%> (+0.50%)`	⬆️
meta0v2/meta0_remote.c	`74.29% <0.00%> (-8.57%)`	⬇️
tests/functional/crawler/test_integrity_crawler.py	`78.85% <0.00%> (-3.84%)`	⬇️
meta1v2/meta1_prefixes.c	`75.78% <0.00%> (-3.09%)`	⬇️
oio/common/http_urllib3.py	`85.00% <0.00%> (-2.50%)`	⬇️
oio/conscience/client.py	`81.82% <0.00%> (-2.47%)`	⬇️
... and 52 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 35366c4...f780d55. Read the comment docs.

murlock · 2020-03-26T10:50:47Z

oio/api/object_storage.py

+        cid = cid or cid_from_name(account, container)
+        cid = cid.upper()
+        return '/'.join(("meta", cid, obj))
+


may be we should add a helper function _del_metadata_cache since lot of code is related to remove entry from cache

murlock · 2020-03-26T10:52:11Z

oio/api/object_storage.py

+                        chunks[float(position)] = chunk
+
+        if meta is None or chunks is None:
+            meta = kwargs.get('meta')


why saving meta if line below it is overwritten ?

It's an oversight, this line should have been deleted.

murlock · 2020-03-26T10:54:29Z

oio/api/object_storage.py

+        meta = None
+        chunks = None
+        cache = kwargs.get('cache')
+        if cache is not None and not version:


if cache and not version ?

If the cache (dict) is empty, this condition remains true.

murlock · 2020-03-26T10:57:53Z

oio/api/object_storage.py

+                                cid=None):
+        if not obj:
+            raise ValueError('Missing object name to use the cache')
+        cid = cid or cid_from_name(account, container)


We have to measure impact of cid_from_name since it will always compute a SHA256

cid_from_name is already used in _object_prepare and object_fetch computes it (unless there was the cid)

murlock · 2020-03-26T10:59:54Z

oio/api/object_storage.py

+        else:
+            chunk_method = meta['chunk_method']
+            storage_method = STORAGE_METHODS.load(chunk_method)
+


It should be nice to split this code in few function but I don't have idea

I moved the cache management to container/client.py, so finally this code is no longer modified.

oio/common/cache.py

fvennetier · 2020-03-27T13:32:07Z

oio/common/cache.py

+    content_meta = content_meta.copy()
+    if properties:
+        metadata['properties'] = content_meta['properties']
+    content_meta['properties'] = dict()


I don't get this. Why is there metadata['properties'] and metadata['meta']['properties']?

By the way, when reading this, it's hard to get what is content_meta and what is metadata...

cache_value['meta']['properties'] is always an empty dictionary. While cache_value['properties'] only exists if the object properties have been retrieved.

cache_value['meta']['properties'] is always an empty dictionary.

Then why do we need it?

tests/functional/api/test_objectstorage.py

fvennetier · 2020-03-31T09:22:34Z

oio/api/object_storage.py

@@ -89,12 +91,15 @@ def __init__(self, namespace, logger=None, perfdata=None, **kwargs):
        conf = {"namespace": self.namespace}
        self.logger = logger or get_logger(conf)
        self.perfdata = perfdata
+        self.cache = cache


No need for this field. Also, I don't think cache needs to appear in this method's signature. Adding 'cache' to EXTRA_KEYWORDS would do the trick.

fvennetier · 2020-03-31T09:26:07Z

oio/common/cache.py

+    content_meta = content_meta.copy()
+    if properties:
+        metadata['properties'] = content_meta['properties']
+    content_meta['properties'] = dict()


cache_value['meta']['properties'] is always an empty dictionary.

Then why do we need it?

AymericDu requested review from murlock and fvennetier March 25, 2020 21:08

AymericDu force-pushed the python-api-allow-to-cache-object-metadata branch from e2147d0 to 8984c29 Compare March 26, 2020 09:05

AymericDu changed the title ~~Python API: Allow to cache object metadata~~ Python API: Allow to cache object metadata and location Mar 26, 2020

murlock approved these changes Mar 26, 2020

View reviewed changes

AymericDu force-pushed the python-api-allow-to-cache-object-metadata branch 4 times, most recently from 3e4f9f1 to f780d55 Compare March 27, 2020 10:38

fvennetier reviewed Mar 27, 2020

View reviewed changes

AymericDu force-pushed the python-api-allow-to-cache-object-metadata branch from f780d55 to 83e7f88 Compare March 27, 2020 15:15

Python API: Allow to cache object metadata and location

6a58d85

AymericDu force-pushed the python-api-allow-to-cache-object-metadata branch from 83e7f88 to 6a58d85 Compare March 27, 2020 15:26

fvennetier approved these changes Mar 27, 2020

View reviewed changes

fvennetier reviewed Mar 31, 2020

View reviewed changes

fvennetier merged commit 9ee11ba into open-io:4.x Mar 31, 2020

AymericDu deleted the python-api-allow-to-cache-object-metadata branch April 2, 2020 07:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python API: Allow to cache object metadata and location #2009

Python API: Allow to cache object metadata and location #2009

AymericDu commented Mar 25, 2020

codecov-io commented Mar 26, 2020 •

edited

Loading

murlock Mar 26, 2020

murlock Mar 26, 2020

AymericDu Mar 26, 2020

murlock Mar 26, 2020

AymericDu Mar 26, 2020

murlock Mar 26, 2020

AymericDu Mar 26, 2020

murlock Mar 26, 2020

AymericDu Mar 26, 2020

fvennetier Mar 27, 2020

AymericDu Mar 27, 2020

fvennetier Mar 31, 2020

fvennetier Mar 31, 2020

fvennetier Mar 31, 2020

Python API: Allow to cache object metadata and location #2009

Python API: Allow to cache object metadata and location #2009

Conversation

AymericDu commented Mar 25, 2020

SUMMARY

ISSUE TYPE

COMPONENT NAME

SDS VERSION

codecov-io commented Mar 26, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Mar 26, 2020 •

edited

Loading