[
https://issues.apache.org/jira/browse/HADOOP-13649?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16452447#comment-16452447
]
Gabor Bota commented on HADOOP-13649:
-------------------------------------
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-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
