You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Darrel Schneider <ds...@pivotal.io> on 2016/11/01 23:25:12 UTC
Review Request 53355: fix deadlock caused by how the GemFireCacheImpl
class synchronization is done
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53355/
-----------------------------------------------------------
Review request for geode, anilkumar gingade and Scott Jewell.
Bugs: GEODE-1971
https://issues.apache.org/jira/browse/GEODE-1971
Repository: geode
Description
-------
shutdownAll now syncs on just the class instead of "this". Since it calls close which also syncs on the class it is best to get this sync done early and hold it for the entire shutdown.
Some other methods that were syncing on the class no longer do. They had not reason for doing so.
Diffs
-----
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java ba4f1f410cef89a276634dadecf46ac80d9c3990
Diff: https://reviews.apache.org/r/53355/diff/
Testing
-------
precheckin
Thanks,
Darrel Schneider
Re: Review Request 53355: fix deadlock caused by how the
GemFireCacheImpl class synchronization is done
Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53355/#review154953
-----------------------------------------------------------
Ship it!
Ship It!
- anilkumar gingade
On Nov. 4, 2016, 5:58 p.m., Darrel Schneider wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53355/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2016, 5:58 p.m.)
>
>
> Review request for geode, anilkumar gingade and Scott Jewell.
>
>
> Bugs: GEODE-1971
> https://issues.apache.org/jira/browse/GEODE-1971
>
>
> Repository: geode
>
>
> Description
> -------
>
> shutdownAll now syncs on just the class instead of "this". Since it calls close which also syncs on the class it is best to get this sync done early and hold it for the entire shutdown.
> Some other methods that were syncing on the class no longer do. They had not reason for doing so.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java ba4f1f410cef89a276634dadecf46ac80d9c3990
> geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/ShutdownAllDUnitTest.java 1bb06f1ce1d93c801a51c1d8eb4f18ffeaa9f5cc
>
> Diff: https://reviews.apache.org/r/53355/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Darrel Schneider
>
>
Re: Review Request 53355: fix deadlock caused by how the
GemFireCacheImpl class synchronization is done
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53355/
-----------------------------------------------------------
(Updated Nov. 4, 2016, 10:58 a.m.)
Review request for geode, anilkumar gingade and Scott Jewell.
Changes
-------
fixed beforeShutdownAll test hook
Bugs: GEODE-1971
https://issues.apache.org/jira/browse/GEODE-1971
Repository: geode
Description
-------
shutdownAll now syncs on just the class instead of "this". Since it calls close which also syncs on the class it is best to get this sync done early and hold it for the entire shutdown.
Some other methods that were syncing on the class no longer do. They had not reason for doing so.
Diffs (updated)
-----
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java ba4f1f410cef89a276634dadecf46ac80d9c3990
geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/ShutdownAllDUnitTest.java 1bb06f1ce1d93c801a51c1d8eb4f18ffeaa9f5cc
Diff: https://reviews.apache.org/r/53355/diff/
Testing
-------
precheckin
Thanks,
Darrel Schneider
Re: Review Request 53355: fix deadlock caused by how the
GemFireCacheImpl class synchronization is done
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53355/#review154942
-----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java (line 1779)
<https://reviews.apache.org/r/53355/#comment224733>
Note that this test hook is now invoked after we have set the new isShutDownAll hook to true.
It use to be invoked before "isShutDownAll" was set to true.
This broke ShutdownAllPersistentGatewaySenderDUnitTest
- Darrel Schneider
On Nov. 3, 2016, 3:16 p.m., Darrel Schneider wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53355/
> -----------------------------------------------------------
>
> (Updated Nov. 3, 2016, 3:16 p.m.)
>
>
> Review request for geode, anilkumar gingade and Scott Jewell.
>
>
> Bugs: GEODE-1971
> https://issues.apache.org/jira/browse/GEODE-1971
>
>
> Repository: geode
>
>
> Description
> -------
>
> shutdownAll now syncs on just the class instead of "this". Since it calls close which also syncs on the class it is best to get this sync done early and hold it for the entire shutdown.
> Some other methods that were syncing on the class no longer do. They had not reason for doing so.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java ba4f1f410cef89a276634dadecf46ac80d9c3990
> geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/ShutdownAllDUnitTest.java 1bb06f1ce1d93c801a51c1d8eb4f18ffeaa9f5cc
>
> Diff: https://reviews.apache.org/r/53355/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Darrel Schneider
>
>
Re: Review Request 53355: fix deadlock caused by how the
GemFireCacheImpl class synchronization is done
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53355/
-----------------------------------------------------------
(Updated Nov. 3, 2016, 3:16 p.m.)
Review request for geode, anilkumar gingade and Scott Jewell.
Changes
-------
Added a unit test that hangs without these changes.
shutDownAll will now, once again, cause a in progress cache creation to close.
Bugs: GEODE-1971
https://issues.apache.org/jira/browse/GEODE-1971
Repository: geode
Description
-------
shutdownAll now syncs on just the class instead of "this". Since it calls close which also syncs on the class it is best to get this sync done early and hold it for the entire shutdown.
Some other methods that were syncing on the class no longer do. They had not reason for doing so.
Diffs (updated)
-----
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java ba4f1f410cef89a276634dadecf46ac80d9c3990
geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/ShutdownAllDUnitTest.java 1bb06f1ce1d93c801a51c1d8eb4f18ffeaa9f5cc
Diff: https://reviews.apache.org/r/53355/diff/
Testing
-------
precheckin
Thanks,
Darrel Schneider
Re: Review Request 53355: fix deadlock caused by how the
GemFireCacheImpl class synchronization is done
Posted by Scott Jewell <sj...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53355/#review154618
-----------------------------------------------------------
Ship it!
Ship It!
- Scott Jewell
On Nov. 1, 2016, 11:25 p.m., Darrel Schneider wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53355/
> -----------------------------------------------------------
>
> (Updated Nov. 1, 2016, 11:25 p.m.)
>
>
> Review request for geode, anilkumar gingade and Scott Jewell.
>
>
> Bugs: GEODE-1971
> https://issues.apache.org/jira/browse/GEODE-1971
>
>
> Repository: geode
>
>
> Description
> -------
>
> shutdownAll now syncs on just the class instead of "this". Since it calls close which also syncs on the class it is best to get this sync done early and hold it for the entire shutdown.
> Some other methods that were syncing on the class no longer do. They had not reason for doing so.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java ba4f1f410cef89a276634dadecf46ac80d9c3990
>
> Diff: https://reviews.apache.org/r/53355/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Darrel Schneider
>
>
Re: Review Request 53355: fix deadlock caused by how the
GemFireCacheImpl class synchronization is done
Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53355/#review154615
-----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java (line 1755)
<https://reviews.apache.org/r/53355/#comment224232>
This changes the behavior of shutdown check.
Earlier if there is concurrent create and shutdown is in progress, the "isCacheAtShutdownAll" is set while create in progress, now it will be set after the create is completed. This will impact all the methods calling "isCacheAtShutdownAll".
- anilkumar gingade
On Nov. 1, 2016, 11:25 p.m., Darrel Schneider wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53355/
> -----------------------------------------------------------
>
> (Updated Nov. 1, 2016, 11:25 p.m.)
>
>
> Review request for geode, anilkumar gingade and Scott Jewell.
>
>
> Bugs: GEODE-1971
> https://issues.apache.org/jira/browse/GEODE-1971
>
>
> Repository: geode
>
>
> Description
> -------
>
> shutdownAll now syncs on just the class instead of "this". Since it calls close which also syncs on the class it is best to get this sync done early and hold it for the entire shutdown.
> Some other methods that were syncing on the class no longer do. They had not reason for doing so.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java ba4f1f410cef89a276634dadecf46ac80d9c3990
>
> Diff: https://reviews.apache.org/r/53355/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Darrel Schneider
>
>