You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "Gabor Bota (JIRA)" <ji...@apache.org> on 2018/04/25 15:18:00 UTC
[jira] [Comment Edited] (HADOOP-13649) s3guard: implement
time-based (TTL) expiry for LocalMetadataStore
[ https://issues.apache.org/jira/browse/HADOOP-13649?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452447#comment-16452447 ]
Gabor Bota edited comment on HADOOP-13649 at 4/25/18 3:17 PM:
--------------------------------------------------------------
Comments on my 001 patch:
1. Removed the LruHasMap implementation
- Using the com.google.common.cache.Cache
- All tests pass
- Loosing the mruGet method. I was not able to find any test for this particular feature, or 'real world' usage besides that it will modify the LinkedList inside the LinkedHashMap, so it will have the most recently modified elements in the start of the List. I don't think that would cause big performance issue in testing, but I've made a little benchmark to test this.[1]
- Using basically the same implementation in LocalMetadataStore, the new addition is only the Timed Eviction.
- Of course I had to rename every hash to cache, and there were some other minor changes.
2. Created a test for the Timed Eviction (testCacheTimedEvictionAfterWrite)
- Only in the TestLocalMetadataStore (not in the abstract test)
- Just tests if the cache implementation inside LocalMetadataStore working as expected, nothing more.
- Maybe additional tests could be added for higher level functionality, but the problem is that there will be a need from the test to change the default cache inside the instance to another cache build with the support for a custom Ticker to avoid realtime waiting for the eviction (which could cause flakyness).
3. Should the removal of the evicted elements be logged?
- There is an option to log removed elements. It can be done synchronously (default), and asynchronously as well. Using the synchronous way could be expensive, and I'm not even sure if we want this (maybe good for debug) so I haven't included it in the initial patch.
4. Maybe DEFAULT_EXPIRY_AFTER_WRITE_SECONDS should be DEFAULT_EVICTION_AFTER_WRITE_SECONDS
[1] The impact of having cache instead of LruHashMap in LocalMetadataStore on tests:
- mvn -Dparallel-tests -DtestsThreadCount=8 clean verify
with patch: 4m10.917s
without patch: 4m12.413s
- mvn -Dparallel-tests -DtestsThreadCount=8 clean test
with patch: 4m6.383s
without patch: 4m5.217s
Test and verify runs on us-west-2 succesfully for the patch.
was (Author: gabor.bota):
Comments on my 001 patch:
1. Removed the LruHasMap implementation
- Using the com.google.common.cache.Cache
- All tests pass
- Loosing the mruGet method. I was not able to find any test for this particular feature,
or 'real world' usage besides that it will modify the LinkedList inside
the LinkedHashMap, so it will have the most recently modified elements
in the start of the List. I don't think that would cause big performance
issue in testing, but I've made a little benchmark to test this.[1]
- Using basically the same implementation in LocalMetadataStore, the
new addition is only the Timed Eviction.
- Of course I had to rename every hash to cache, and there were some other minor changes.
2. Created a test for the Timed Eviction (testCacheTimedEvictionAfterWrite)
- Only in the TestLocalMetadataStore (not in the abstract test)
- Just tests if the cache implementation inside LocalMetadataStore working
as expected, nothing more.
- Maybe additional tests could be added for higher level functionality, but the
problem is that there will be a need from the test to change the default
cache inside the instance to another cache build with the support for a custom Ticker
to avoid realtime waiting for the eviction (which could cause flakyness).
3. Should the removal of the evicted elements be logged?
- There is an option to log removed elements. It can be done synchronously (default),
and asynchronously as well. Using the synchronous way could be expensive, and I'm
not even sure if we want this (maybe good for debug) so I haven't included it in the
initial patch.
4. Maybe DEFAULT_EXPIRY_AFTER_WRITE_SECONDS should be DEFAULT_EVICTION_AFTER_WRITE_SECONDS
[1] The impact of having cache instead of LruHashMap in LocalMetadataStore on tests:
- mvn -Dparallel-tests -DtestsThreadCount=8 clean verify
with patch: 4m10.917s
without patch: 4m12.413s
- mvn -Dparallel-tests -DtestsThreadCount=8 clean test
with patch: 4m6.383s
without patch: 4m5.217s
Test and verify runs on us-west-2 succesfully for the patch.
> s3guard: implement time-based (TTL) expiry for LocalMetadataStore
> -----------------------------------------------------------------
>
> Key: HADOOP-13649
> URL: https://issues.apache.org/jira/browse/HADOOP-13649
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Affects Versions: 3.0.0-beta1
> Reporter: Aaron Fabbri
> Assignee: Gabor Bota
> Priority: Minor
> Attachments: HADOOP-13649.001.patch
>
>
> LocalMetadataStore is primarily a reference implementation for testing. It may be useful in narrow circumstances where the workload can tolerate short-term lack of inter-node consistency: Being in-memory, one JVM/node's LocalMetadataStore will not see another node's changes to the underlying filesystem.
> To put a bound on the time during which this inconsistency may occur, we should implement time-based (a.k.a. Time To Live / TTL) expiration for LocalMetadataStore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org