You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Charles Lee <li...@gmail.com> on 2010/11/11 06:13:44 UTC
Re: [jira] Created: (HARMONY-6675) Reducing timeout value in
CanonicalPatchCache to fix a file not found error in Hadoop common
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)
<ji...@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.
>
>
--
Yours sincerely,
Charles Lee
Re: [jira] Created: (HARMONY-6675) Reducing timeout value in
CanonicalPatchCache to fix a file not found error in Hadoop common
Posted by regis xu <xu...@gmail.com>.
I just committed the patch at r1035930.
On Wed, Nov 17, 2010 at 9:49 AM, Regis <xu...@gmail.com> wrote:
> On 2010-11-16 18:40, sebb wrote:
>
>> On 16 November 2010 06:56, Regis<xu...@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)
>>>>> <ji...@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.
>
Re: [jira] Created: (HARMONY-6675) Reducing timeout value in
CanonicalPatchCache to fix a file not found error in Hadoop common
Posted by sebb <se...@gmail.com>.
On 18 November 2010 03:01, Regis <xu...@gmail.com> wrote:
> On 2010-11-18 4:05, sebb wrote:
>>
>> Just had a look at the code.
>>
>> Effectively the timeout field is a public mutable static variable.
>> Access to the field is not synchronised at all.
>>
>> So if one thread sets a new value, other threads may or may not see
>> the updated value (ever).
>> Worse, unless the JVM guarantees that writes to long variables are
>> atomic, AFAICT if two threads write the value, then it's possible that
>> the timeout field could end up with a value that is equal to neither
>> setting.
>>
>> Making the field volatile should fix these problems - I think.
>
> Yes, setTimeout is not treahd safe here, in my understanding, volatile could
> make sure threads read the latest value, but to resolve write conflict,
> synchronization is still needed.
>
>>
>> There is a subtle bug - if the property sets the timeout to<=0, then
>> isEnable is set to false. setTimeout() never sets it true.
>>
>> Furthermore, since access to isEnable is not synch. either, if one
>> thread sets timeout 0 and the another to non-zero, isValid could end
>> up being incorrect for the value of the timeout. It's always risky
>> when two fields are interdependent like this.
>>
>> It would be safer to eliminate the isValid field and just check the
>> value of the timeout.
>
> True, field 'isEnable' is not necessary, will remove it.
>
>>
>> BTW, the private instance fields in CacheElement should be final; that
>> would make the class immutable and thread-safe.
>>
>
> Go through the code, I also make field 'cache', 'list' and 'lock' to final.
>
> So [1] is a new patch according Sebb's comments:
>
> 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
> @@ -29,9 +29,9 @@ import java.util.LinkedList;
> public class FileCanonPathCache {
>
> static private class CacheElement {
> - String canonicalPath;
> + final String canonicalPath;
>
> - long timestamp;
> + final long timestamp;
>
> public CacheElement(String path) {
> this.canonicalPath = path;
> @@ -44,25 +44,20 @@ public class FileCanonPathCache {
> */
> public static final int CACHE_SIZE = 256;
>
> - private static HashMap<String, CacheElement> cache = new
> HashMap<String, CacheElement>(
> + private static final HashMap<String, CacheElement> cache = new
> HashMap<String, CacheElement>(
> CACHE_SIZE);
>
> /**
> * FIFO queue for tracking age of elements.
> */
> - private static LinkedList<String> list = new LinkedList<String>();
> + private static final LinkedList<String> list = new
> LinkedList<String>();
>
> - private static Object lock = new Object();
> + private static final Object lock = new Object();
>
> /**
> * Expired time, 0 disable this cache.
> */
> - private static long timeout = 30000;
> -
> - /**
> - * Whether to enable this cache.
> - */
> - private static boolean isEnable = true;
> + private static volatile long timeout = 30000;
>
> public static final String FILE_CANONICAL_PATH_CACHE_TIMEOUT =
> "org.apache.harmony.file.canonical.path.cache.timeout";
>
> @@ -74,8 +69,8 @@ public class FileCanonPathCache {
> // use default timeout value
> }
>
> - if (timeout <= 0) {
> - isEnable = false;
> + if (timeout < 0) {
> + timeout = 0;
> }
> }
>
> @@ -88,7 +83,8 @@ public class FileCanonPathCache {
> *
> */
> public static String get(String path) {
> - if (!isEnable) {
> + long localTimeout = timeout;
> + if (localTimeout == 0) {
> return null;
> }
>
> @@ -102,7 +98,7 @@ public class FileCanonPathCache {
> }
>
> long time = System.currentTimeMillis();
> - if (time - element.timestamp > timeout) {
> + if (time - element.timestamp > localTimeout) {
> // remove all elements older than this one
> synchronized (lock) {
> if (cache.get(path) != null) {
> @@ -128,7 +124,7 @@ public class FileCanonPathCache {
> * the canonical path of <code>path</code>.
> */
> public static void put(String path, String canonicalPath) {
> - if (!isEnable) {
> + if (timeout == 0) {
> return;
> }
>
> @@ -148,7 +144,7 @@ public class FileCanonPathCache {
> * Remove all elements from cache.
> */
> public static void clear() {
> - if (!isEnable) {
> + if (timeout == 0) {
> return;
> }
>
> @@ -163,10 +159,12 @@ public class FileCanonPathCache {
> }
>
> public static void setTimeout(long timeout) {
> - FileCanonPathCache.timeout = timeout;
> - if (timeout <= 0) {
That was correct.
> - clear();
> - isEnable = false;
> + synchronized (lock) {
> + if (timeout < 0) {
That is incorrect; should be
if (timeout <= 0) {
otherwise you won't call clear() for timeout == 0.
> + timeout = 0;
> + clear();
> + }
> + FileCanonPathCache.timeout = timeout;
> }
> }
> }
>
Otherwise patch looks good.
Re: [jira] Created: (HARMONY-6675) Reducing timeout value in CanonicalPatchCache
to fix a file not found error in Hadoop common
Posted by Regis <xu...@gmail.com>.
On 2010-11-18 4:05, sebb wrote:
>
> Just had a look at the code.
>
> Effectively the timeout field is a public mutable static variable.
> Access to the field is not synchronised at all.
>
> So if one thread sets a new value, other threads may or may not see
> the updated value (ever).
> Worse, unless the JVM guarantees that writes to long variables are
> atomic, AFAICT if two threads write the value, then it's possible that
> the timeout field could end up with a value that is equal to neither
> setting.
>
> Making the field volatile should fix these problems - I think.
Yes, setTimeout is not treahd safe here, in my understanding, volatile could
make sure threads read the latest value, but to resolve write conflict,
synchronization is still needed.
>
> There is a subtle bug - if the property sets the timeout to<=0, then
> isEnable is set to false. setTimeout() never sets it true.
>
> Furthermore, since access to isEnable is not synch. either, if one
> thread sets timeout 0 and the another to non-zero, isValid could end
> up being incorrect for the value of the timeout. It's always risky
> when two fields are interdependent like this.
>
> It would be safer to eliminate the isValid field and just check the
> value of the timeout.
True, field 'isEnable' is not necessary, will remove it.
>
> BTW, the private instance fields in CacheElement should be final; that
> would make the class immutable and thread-safe.
>
Go through the code, I also make field 'cache', 'list' and 'lock' to final.
So [1] is a new patch according Sebb's comments:
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
@@ -29,9 +29,9 @@ import java.util.LinkedList;
public class FileCanonPathCache {
static private class CacheElement {
- String canonicalPath;
+ final String canonicalPath;
- long timestamp;
+ final long timestamp;
public CacheElement(String path) {
this.canonicalPath = path;
@@ -44,25 +44,20 @@ public class FileCanonPathCache {
*/
public static final int CACHE_SIZE = 256;
- private static HashMap<String, CacheElement> cache = new HashMap<String,
CacheElement>(
+ private static final HashMap<String, CacheElement> cache = new
HashMap<String, CacheElement>(
CACHE_SIZE);
/**
* FIFO queue for tracking age of elements.
*/
- private static LinkedList<String> list = new LinkedList<String>();
+ private static final LinkedList<String> list = new LinkedList<String>();
- private static Object lock = new Object();
+ private static final Object lock = new Object();
/**
* Expired time, 0 disable this cache.
*/
- private static long timeout = 30000;
-
- /**
- * Whether to enable this cache.
- */
- private static boolean isEnable = true;
+ private static volatile long timeout = 30000;
public static final String FILE_CANONICAL_PATH_CACHE_TIMEOUT =
"org.apache.harmony.file.canonical.path.cache.timeout";
@@ -74,8 +69,8 @@ public class FileCanonPathCache {
// use default timeout value
}
- if (timeout <= 0) {
- isEnable = false;
+ if (timeout < 0) {
+ timeout = 0;
}
}
@@ -88,7 +83,8 @@ public class FileCanonPathCache {
*
*/
public static String get(String path) {
- if (!isEnable) {
+ long localTimeout = timeout;
+ if (localTimeout == 0) {
return null;
}
@@ -102,7 +98,7 @@ public class FileCanonPathCache {
}
long time = System.currentTimeMillis();
- if (time - element.timestamp > timeout) {
+ if (time - element.timestamp > localTimeout) {
// remove all elements older than this one
synchronized (lock) {
if (cache.get(path) != null) {
@@ -128,7 +124,7 @@ public class FileCanonPathCache {
* the canonical path of <code>path</code>.
*/
public static void put(String path, String canonicalPath) {
- if (!isEnable) {
+ if (timeout == 0) {
return;
}
@@ -148,7 +144,7 @@ public class FileCanonPathCache {
* Remove all elements from cache.
*/
public static void clear() {
- if (!isEnable) {
+ if (timeout == 0) {
return;
}
@@ -163,10 +159,12 @@ public class FileCanonPathCache {
}
public static void setTimeout(long timeout) {
- FileCanonPathCache.timeout = timeout;
- if (timeout <= 0) {
- clear();
- isEnable = false;
+ synchronized (lock) {
+ if (timeout < 0) {
+ timeout = 0;
+ clear();
+ }
+ FileCanonPathCache.timeout = timeout;
}
}
}
--
Best Regards,
Regis.
Re: [jira] Created: (HARMONY-6675) Reducing timeout value in
CanonicalPatchCache to fix a file not found error in Hadoop common
Posted by sebb <se...@gmail.com>.
On 17 November 2010 01:49, Regis <xu...@gmail.com> wrote:
> On 2010-11-16 18:40, sebb wrote:
>>
>> On 16 November 2010 06:56, Regis<xu...@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)
>>>>> <ji...@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'.
Just had a look at the code.
Effectively the timeout field is a public mutable static variable.
Access to the field is not synchronised at all.
So if one thread sets a new value, other threads may or may not see
the updated value (ever).
Worse, unless the JVM guarantees that writes to long variables are
atomic, AFAICT if two threads write the value, then it's possible that
the timeout field could end up with a value that is equal to neither
setting.
Making the field volatile should fix these problems - I think.
There is a subtle bug - if the property sets the timeout to <=0, then
isEnable is set to false. setTimeout() never sets it true.
Furthermore, since access to isEnable is not synch. either, if one
thread sets timeout 0 and the another to non-zero, isValid could end
up being incorrect for the value of the timeout. It's always risky
when two fields are interdependent like this.
It would be safer to eliminate the isValid field and just check the
value of the timeout.
BTW, the private instance fields in CacheElement should be final; that
would make the class immutable and thread-safe.
>>
>>> +
>>> + /**
>>> + * 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.
>
Re: [jira] Created: (HARMONY-6675) Reducing timeout value in CanonicalPatchCache
to fix a file not found error in Hadoop common
Posted by Regis <xu...@gmail.com>.
On 2010-11-16 18:40, sebb wrote:
> On 16 November 2010 06:56, Regis<xu...@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)
>>>> <ji...@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.
Re: [jira] Created: (HARMONY-6675) Reducing timeout value in
CanonicalPatchCache to fix a file not found error in Hadoop common
Posted by sebb <se...@gmail.com>.
On 16 November 2010 06:56, Regis <xu...@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)
>>> <ji...@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).
> +
> + /**
> + * 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.
>
Re: [jira] Created: (HARMONY-6675) Reducing timeout value in CanonicalPatchCache
to fix a file not found error in Hadoop common
Posted by Regis <xu...@gmail.com>.
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)
>> <ji...@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;
+
+ /**
+ * Whether to enable this cache.
+ */
+ private static boolean isEnable = true;
+
+ 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.
Re: [jira] Created: (HARMONY-6675) Reducing timeout value in CanonicalPatchCache
to fix a file not found error in Hadoop common
Posted by Regis <xu...@gmail.com>.
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)
> <ji...@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?
--
Best Regards,
Regis.