[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Tom White (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15110485#comment-15110485
 ] 

Tom White commented on AVRO-1781:
-

The intent was to put this into 1.8 - I've just backported it.

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Assignee: Ryan Blue
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781-ADDENDUM.1.patch, AVRO-1781-ADDENDUM.2.patch, 
> AVRO-1781.1.patch, AVRO-1781.2.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15110484#comment-15110484
 ] 

ASF subversion and git services commented on AVRO-1781:
---

Commit 1725916 from tomwh...@apache.org in branch 'avro/branches/branch-1.8'
[ https://svn.apache.org/r1725916 ]

AVRO-1781 ADDENDUM: Remove LogicalTypes cache.

This cache maps a Schema to its corresponding LogicalType that has been
constructed from the Schema's properties, by object identity. This is no
longer needed now that the LogicalType is set on the schema instance
when it is parsed. The cache was necessary when we were trying to avoid
all changes to the Schema class.

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Assignee: Ryan Blue
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781-ADDENDUM.1.patch, AVRO-1781-ADDENDUM.2.patch, 
> AVRO-1781.1.patch, AVRO-1781.2.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Sean Busbey (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15107915#comment-15107915
 ] 

Sean Busbey commented on AVRO-1781:
---

is the addendum meant to be applied after reverting the previous patch?

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Assignee: Ryan Blue
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781-ADDENDUM.1.patch, AVRO-1781.1.patch, 
> AVRO-1781.2.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Ryan Blue (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15107965#comment-15107965
 ] 

Ryan Blue commented on AVRO-1781:
-

Thanks for having a look, Sean!

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Assignee: Ryan Blue
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781-ADDENDUM.1.patch, AVRO-1781-ADDENDUM.2.patch, 
> AVRO-1781.1.patch, AVRO-1781.2.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Ryan Blue (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15107916#comment-15107916
 ] 

Ryan Blue commented on AVRO-1781:
-

No, the previous patch was still needed since we are moving to guava for other 
caches. AVRO-1760 is based on the other changes in that patch. It just happens 
that this cache isn't needed.

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Assignee: Ryan Blue
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781-ADDENDUM.1.patch, AVRO-1781.1.patch, 
> AVRO-1781.2.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Sean Busbey (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15107920#comment-15107920
 ] 

Sean Busbey commented on AVRO-1781:
---

{code}
import com.google.common.base.Optional;
import com.google.common.collect.MapMaker;
{code}

Both of these imports are no longer needed.

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Assignee: Ryan Blue
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781-ADDENDUM.1.patch, AVRO-1781.1.patch, 
> AVRO-1781.2.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Sean Busbey (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15107950#comment-15107950
 ] 

Sean Busbey commented on AVRO-1781:
---

+1

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Assignee: Ryan Blue
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781-ADDENDUM.1.patch, AVRO-1781-ADDENDUM.2.patch, 
> AVRO-1781.1.patch, AVRO-1781.2.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Hudson (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15107993#comment-15107993
 ] 

Hudson commented on AVRO-1781:
--

SUCCESS: Integrated in AvroJava #572 (See 
[https://builds.apache.org/job/AvroJava/572/])
AVRO-1781 ADDENDUM: Remove LogicalTypes cache.

This cache maps a Schema to its corresponding LogicalType that has been
constructed from the Schema's properties, by object identity. This is no
longer needed now that the LogicalType is set on the schema instance
when it is parsed. The cache was necessary when we were trying to avoid
all changes to the Schema class. (blue: rev 1725668)
* trunk/lang/java/avro/src/main/java/org/apache/avro/LogicalTypes.java


> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Assignee: Ryan Blue
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781-ADDENDUM.1.patch, AVRO-1781-ADDENDUM.2.patch, 
> AVRO-1781.1.patch, AVRO-1781.2.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15098014#comment-15098014
 ] 

ASF subversion and git services commented on AVRO-1781:
---

Commit 1724591 from tomwh...@apache.org in branch 'avro/branches/branch-1.8'
[ https://svn.apache.org/r1724591 ]

AVRO-1781: Use Guava for a thread-safe weak identity cache.

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781.1.patch, AVRO-1781.2.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Tom White (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15096051#comment-15096051
 ] 

Tom White commented on AVRO-1781:
-

The patch now works for me. Overall I'm +1 on this. Could you add a comment to 
the line where the Guava version number is defined to document the restrictions 
on changing it.

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781.1.patch, AVRO-1781.2.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15096712#comment-15096712
 ] 

ASF subversion and git services commented on AVRO-1781:
---

Commit 1724473 from [~b...@cloudera.com] in branch 'avro/trunk'
[ https://svn.apache.org/r1724473 ]

AVRO-1781: Use Guava for a thread-safe weak identity cache.

This adds avro-guava-dependencies to minimize just Guava classes that
are shaded without minimizing classes from other dependencies. This
filtered artifact is then shaded and relocated in the avro module to
avoid leaking Guava classes to downstream consumers.

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781.1.patch, AVRO-1781.2.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Ryan Blue (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15096719#comment-15096719
 ] 

Ryan Blue commented on AVRO-1781:
-

Thanks, Tom! I've added the comment and committed this.

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781.1.patch, AVRO-1781.2.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: [jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Doug Cutting]

Should we (in separate issues) remove other uses of
WeakIdentityHashMap?  Are there other caches that might be improved
with Guava?  For example, GenericDatumReader's RESOLVER_CACHE might
avoid ThreadLocal.

Doug

On Wed, Jan 13, 2016 at 10:18 AM, Ryan Blue (JIRA)  wrote:
>
> [ 
> https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15096719#comment-15096719
>  ]
>
> Ryan Blue commented on AVRO-1781:
> -
>
> Thanks, Tom! I've added the comment and committed this.
>
>> Schema.parse is not thread safe
>> ---
>>
>> Key: AVRO-1781
>> URL: https://issues.apache.org/jira/browse/AVRO-1781
>> Project: Avro
>>  Issue Type: Bug
>>  Components: java
>>Affects Versions: 1.8.0
>>Reporter: Sean Busbey
>>Priority: Blocker
>> Fix For: 1.8.0
>>
>> Attachments: AVRO-1781.1.patch, AVRO-1781.2.patch
>>
>>
>> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
>> on any schema that is expressed as a JSON object (anything except bare 
>> primitives).
>> That static method relies on a static cache based on WeakIdentityHashMap 
>> (WIHM).
>> WIHM clearly states that it isn't threadsafe 
>> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
>> {code}
>>  *
>>  * Note that this implementation is not synchronized.
>>  * 
>>  */
>> public class WeakIdentityHashMap implements Map {
>> {code}
>> All of the Schema.Parser instances use that same static Schema.parse method.
>> The end result is that as-is it's only safe to have a single thread parsing 
>> schemas in a given JVM.
>
>
>
> --
> This message was sent by Atlassian JIRA
> (v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Hudson (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15096788#comment-15096788
 ] 

Hudson commented on AVRO-1781:
--

SUCCESS: Integrated in AvroJava #568 (See 
[https://builds.apache.org/job/AvroJava/568/])
AVRO-1781: Use Guava for a thread-safe weak identity cache.

This adds avro-guava-dependencies to minimize just Guava classes that
are shaded without minimizing classes from other dependencies. This
filtered artifact is then shaded and relocated in the avro module to
avoid leaking Guava classes to downstream consumers. (blue: rev 1724473)
* trunk/CHANGES.txt
* trunk/lang/java/avro/pom.xml
* trunk/lang/java/avro/src/main/java/org/apache/avro/LogicalTypes.java
* trunk/lang/java/avro/src/main/resources
* trunk/lang/java/avro/src/main/resources/META-INF
* trunk/lang/java/avro/src/main/resources/META-INF/LICENSE
* trunk/lang/java/guava
* trunk/lang/java/guava/pom.xml
* trunk/lang/java/guava/src
* trunk/lang/java/guava/src/main
* trunk/lang/java/guava/src/main/java
* trunk/lang/java/guava/src/main/java/org
* trunk/lang/java/guava/src/main/java/org/apache
* trunk/lang/java/guava/src/main/java/org/apache/avro
* trunk/lang/java/guava/src/main/java/org/apache/avro/GuavaClasses.java
* trunk/lang/java/guava/src/main/resources
* trunk/lang/java/guava/src/main/resources/META-INF
* trunk/lang/java/guava/src/main/resources/META-INF/LICENSE
* trunk/lang/java/mapred/pom.xml
* trunk/lang/java/pom.xml
* trunk/lang/java/tools/src/main/resources/META-INF/LICENSE


> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781.1.patch, AVRO-1781.2.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: [jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Ryan Blue]

Yes, definitely. AVRO-1760 is one that is waiting on this commit to do 
basically the same fix. I think we should remove the custom Identity 
maps and replace them with Guava equivalents.


rb

On 01/13/2016 10:55 AM, Doug Cutting wrote:

Should we (in separate issues) remove other uses of
WeakIdentityHashMap?  Are there other caches that might be improved
with Guava?  For example, GenericDatumReader's RESOLVER_CACHE might
avoid ThreadLocal.

Doug

On Wed, Jan 13, 2016 at 10:18 AM, Ryan Blue (JIRA)  wrote:


 [ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15096719#comment-15096719
 ]

Ryan Blue commented on AVRO-1781:
-

Thanks, Tom! I've added the comment and committed this.


Schema.parse is not thread safe
---

 Key: AVRO-1781
 URL: https://issues.apache.org/jira/browse/AVRO-1781
 Project: Avro
  Issue Type: Bug
  Components: java
Affects Versions: 1.8.0
Reporter: Sean Busbey
Priority: Blocker
 Fix For: 1.8.0

 Attachments: AVRO-1781.1.patch, AVRO-1781.2.patch


Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} on 
any schema that is expressed as a JSON object (anything except bare primitives).
That static method relies on a static cache based on WeakIdentityHashMap (WIHM).
WIHM clearly states that it isn't threadsafe 
[ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
{code}
  *
  * Note that this implementation is not synchronized.
  * 
  */
public class WeakIdentityHashMap implements Map {
{code}
All of the Schema.Parser instances use that same static Schema.parse method.
The end result is that as-is it's only safe to have a single thread parsing 
schemas in a given JVM.




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)



--
Ryan Blue
Software Engineer
Cloudera, Inc.

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Ryan Blue (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15094503#comment-15094503
 ] 

Ryan Blue commented on AVRO-1781:
-

The problem is that Guava's MapMaker maps don't allow Null. I've updated the 
patch to use Optional and tests are passing.

I also ran into trouble with the mapred module because the Guava dependency, 
despite being shaded, was overriding Hadoop's guava dependency. I've added a 
ban for all versions of Guava other than 11.0.2 and for avro-guava-dependencies 
(to make sure it doesn't leak Guava classes into the classpath). Unfortunately, 
the only way to avoid hitting the Guava ban is to use version 11.0.2. If I use 
19.0 and build/test from the mapred directory it correctly uses 11.0.2, but if 
I build from the lang/java directory the dependencies are all resolved at once 
and 19.0 overrides the transitive dependency's version. Luckily, everything 
works with 11.0.2 and the jar is even a little smaller.

I also had to update this to exclude Google's JSR301 jar, which might be GPL 
and is banned by avro-tools. I'm attaching a new patch.

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Priority: Blocker
> Fix For: 1.8.0
>
> Attachments: AVRO-1781.1.patch
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Sean Busbey (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15092283#comment-15092283
 ] 

Sean Busbey commented on AVRO-1781:
---

2 main approaches I see:

* externalize the cache and store one per Schema.Parser instance, so you get 
caching if you reuse and the app can decide on parallelism trade-offs
* switch to using a R/W lock based cache so that it is threadsafe.

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Priority: Blocker
> Fix For: 1.8.0
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (AVRO-1781) Schema.parse is not thread safe

[Ryan Blue (JIRA)]

[ 
https://issues.apache.org/jira/browse/AVRO-1781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15092454#comment-15092454
 ] 

Ryan Blue commented on AVRO-1781:
-

I think this is the same as AVRO-1773. Another option is to use a better weak 
hash map from guava that supports concurrency. That's the solution recommended 
for AVRO-1760 as well.

I have reservations about adding a dependency on Guava since the suggestion is 
currently to shade it, but it would avoid many issues like this one.

> Schema.parse is not thread safe
> ---
>
> Key: AVRO-1781
> URL: https://issues.apache.org/jira/browse/AVRO-1781
> Project: Avro
>  Issue Type: Bug
>  Components: java
>Affects Versions: 1.8.0
>Reporter: Sean Busbey
>Priority: Blocker
> Fix For: 1.8.0
>
>
> Post AVRO-1497, Schema.parse calls {{LogicalTypes.fromSchemaIgnoreInvalid}} 
> on any schema that is expressed as a JSON object (anything except bare 
> primitives).
> That static method relies on a static cache based on WeakIdentityHashMap 
> (WIHM).
> WIHM clearly states that it isn't threadsafe 
> [ref|https://github.com/apache/avro/blob/branch-1.8/lang/java/avro/src/main/java/org/apache/avro/util/WeakIdentityHashMap.java#L42]
> {code}
>  * 
>  * Note that this implementation is not synchronized.
>  * 
>  */
> public class WeakIdentityHashMap implements Map {
> {code}
> All of the Schema.Parser instances use that same static Schema.parse method.
> The end result is that as-is it's only safe to have a single thread parsing 
> schemas in a given JVM.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)