I just committed the patch at r1035930. On Wed, Nov 17, 2010 at 9:49 AM, Regis <xu.re...@gmail.com> wrote:
> On 2010-11-16 18:40, sebb wrote: > >> On 16 November 2010 06:56, Regis<xu.re...@gmail.com> wrote: >> >>> On 2010-11-11 13:26, Regis wrote: >>> >>>> >>>> On 2010-11-11 13:13, Charles Lee wrote: >>>> >>>>> >>>>> The cache is good for the performance. But when the cache meet link, >>>>> there >>>>> will be some difficult situation. The patch has attached is a >>>>> workaround for >>>>> the specified test case. The good choose for the timeout should bed >>>>> discussed. >>>>> >>>>> On Thu, Nov 11, 2010 at 7:30 AM, Guillermo Cabrera (JIRA) >>>>> <j...@apache.org>wrote: >>>>> >>>>> Reducing timeout value in CanonicalPatchCache to fix a file not found >>>>>> error >>>>>> in Hadoop common >>>>>> >>>>>> >>>>>> >>>>>> -------------------------------------------------------------------------------------------- >>>>>> >>>>>> >>>>>> Key: HARMONY-6675 >>>>>> URL: https://issues.apache.org/jira/browse/HARMONY-6675 >>>>>> Project: Harmony >>>>>> Issue Type: New Feature >>>>>> Environment: SLE v. 11, Apache Harmony 6 >>>>>> Reporter: Guillermo Cabrera >>>>>> Priority: Minor >>>>>> >>>>>> >>>>>> Testing Harmony Select (r1022137) with Hadoop common, we ran across >>>>>> the >>>>>> following error: >>>>>> >>>>>> java.lang.RuntimeException: Error while running command to get file >>>>>> permissions : org.apache.hadoop.util.Shell$ExitCodeException: /bin/ls: >>>>>> /tmp/test1/file: No such file or directory >>>>>> >>>>>> Charles Lee (Apache Harmony developer) provided us with the following >>>>>> answer: >>>>>> >>>>>> "For all the test case failures in >>>>>> org.apache.hadoop.fs.TestLocalFSFileContextSymlink, the root cause is >>>>>> we >>>>>> have a CanonicalPathCache under the File, so the file canonical path >>>>>> will be >>>>>> wrong if the test case highly stressed, (I remember the cache time is >>>>>> set to >>>>>> 10 minute)." >>>>>> >>>>>> His patch to fix this issue has been attached. >>>>>> >>>>>> -- >>>>>> This message is automatically generated by JIRA. >>>>>> - >>>>>> You can reply to this email to add a comment to the issue online. >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>> I think we should provide a way to configure this cache: enable/disable >>>> it and timeout, something like >>>> -Dorg.apache.harmony.canonpath.cache.timeout=60, this property could be >>>> overwritten at run time. >>>> >>>> Also the default value of timeout, 10 minutes seems too long, maybe >>>> reduce to one minute or half minute is reasonable for the most of >>>> applications? >>>> >>>> >>> I propose following patch, which configure canonical path cache via >>> system >>> property 'org.apache.harmony.file.canonical.path.cache.timeout', set it >>> to >>> zero could disable the cache completely. And I also reduce default >>> timeout >>> from 10 minutes to 30 seconds. If no one object, I'll commit this path >>> soon. >>> >>> >>> Index: >>> >>> modules/luni/src/main/java/org/apache/harmony/luni/internal/io/FileCanonPathCache.java >>> ===================================================================== >>> --- >>> >>> modules/luni/src/main/java/org/apache/harmony/luni/internal/io/FileCanonPathCache.java >>> +++ >>> >>> modules/luni/src/main/java/org/apache/harmony/luni/internal/io/FileCanonPathCache.java >>> @@ -55,9 +55,29 @@ public class FileCanonPathCache { >>> private static Object lock = new Object(); >>> >>> /** >>> - * Expired time. >>> + * Expired time, 0 disable this cache. >>> */ >>> - private static long timeout = 600000; >>> + private static long timeout = 30000; >>> >> >> Static variables should really be final (even if private). >> > > This variable can't be 'final' because we provide a method setTimeout() > which set a new value of 'timeout'. > > > >> + >>> + /** >>> + * Whether to enable this cache. >>> + */ >>> + private static boolean isEnable = true; >>> + >>> >> >> Likewise. >> >> + public static final String FILE_CANONICAL_PATH_CACHE_TIMEOUT = >>> "org.apache.harmony.file.canonical.path.cache.timeout"; >>> + >>> + static { >>> + String value = >>> System.getProperty(FILE_CANONICAL_PATH_CACHE_TIMEOUT); >>> + try { >>> + timeout = Long.parseLong(value); >>> + } catch (NumberFormatException e) { >>> + // use default timeout value >>> + } >>> + >>> + if (timeout<= 0) { >>> + isEnable = false; >>> + } >>> + } >>> >>> /** >>> * Retrieve element from cache. >>> @@ -68,6 +88,10 @@ public class FileCanonPathCache { >>> * >>> */ >>> public static String get(String path) { >>> + if (!isEnable) { >>> + return null; >>> + } >>> + >>> CacheElement element = null; >>> synchronized (lock) { >>> element = cache.get(path); >>> @@ -104,6 +128,10 @@ public class FileCanonPathCache { >>> * the canonical path of<code>path</code>. >>> */ >>> public static void put(String path, String canonicalPath) { >>> + if (!isEnable) { >>> + return; >>> + } >>> + >>> CacheElement element = new CacheElement(canonicalPath); >>> synchronized (lock) { >>> if (cache.size()>= CACHE_SIZE) { >>> @@ -120,6 +148,10 @@ public class FileCanonPathCache { >>> * Remove all elements from cache. >>> */ >>> public static void clear() { >>> + if (!isEnable) { >>> + return; >>> + } >>> + >>> synchronized (lock) { >>> cache.clear(); >>> list.clear(); >>> @@ -132,5 +164,8 @@ public class FileCanonPathCache { >>> >>> public static void setTimeout(long timeout) { >>> FileCanonPathCache.timeout = timeout; >>> + if (timeout<= 0) { >>> + isEnable = false; >>> + } >>> } >>> } >>> >>> >>> -- >>> Best Regards, >>> Regis. >>> >>> >> > > -- > Best Regards, > Regis. >
