You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "kirklund (GitHub)" <gi...@apache.org> on 2019/02/05 01:01:29 UTC
[GitHub] [geode] kirklund opened pull request #3161: GEODE-6033: Add
VMEventListener for DUnit Rules
A DUnit Rule (or even a test) can now register a VMEventListener:
`VM.getVMEventListenerRegistry().addVMEventListener(vmEventListener);`
VMEventListeners will receive the following notifications:
```
public void afterCreateVM(VM vm);
public void beforeBounceVM(VM vm);
public void afterBounceVM(VM vm);
```
DUnit Rules can use these callbacks in order to support
dynamic creation or bouncing of VMs.
Note: this PR does not complete the ticket GEODE-6033. Next step is to change the DUnit Rules to register a VMEventListener. Then the ticket will be resolved.
[ Full content available at: https://github.com/apache/geode/pull/3161 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund commented on pull request #3161:
GEODE-6033: Add VMEventListener for DUnit Rules
Posted by "kirklund (GitHub)" <gi...@apache.org>.
This is to prevent concurrent modification exception if another thread adds/removes a listener while a notification is occurring.
[ Full content available at: https://github.com/apache/geode/pull/3161 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] upthewaterspout commented on pull request #3161:
GEODE-6033: Add VMEventListener for DUnit Rules
Posted by "upthewaterspout (GitHub)" <gi...@apache.org>.
This won't prevent concurrent modification at all. The array list constructor also iterates over the listeners without synchronization!
Use a CopyOnWriteArrayList for the listeners if you want this to be threadsafe, and make sure the constructor that takes a list *copies* the user's list into it.
[ Full content available at: https://github.com/apache/geode/pull/3161 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund commented on pull request #3161:
GEODE-6033: Add VMEventListener for DUnit Rules
Posted by "kirklund (GitHub)" <gi...@apache.org>.
Good catch. Thanks!
[ Full content available at: https://github.com/apache/geode/pull/3161 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] upthewaterspout commented on pull request #3161:
GEODE-6033: Add VMEventListener for DUnit Rules
Posted by "upthewaterspout (GitHub)" <gi...@apache.org>.
Technically, someone would invoke this constructor with an immutable list, which would be a problem for your add/remove methods below. Maybe copy the passed in list if you need this constructor?
[ Full content available at: https://github.com/apache/geode/pull/3161 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund commented on issue #3161: GEODE-6033: Add
VMEventListener for DUnit Rules
Posted by "kirklund (GitHub)" <gi...@apache.org>.
Failures:
* JGroupsMessengerJUnitTest.testMulticastTest
* LocatorUDPSecurityDUnitTest.testMultipleLocatorsRestartingAtSameTimeWithMissingServers
* CacheXml70DUnitTest.testPARTITION_OVERFLOW
* CacheXml81DUnitTest.testPARTITION_OVERFLOW
* ClientServerMiscBCDUnitTest.giiEventQueueFromOldToCurrentMemberShouldSucceed[3]
None of these seem to be related to this PR.
[ Full content available at: https://github.com/apache/geode/pull/3161 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund closed pull request #3161: GEODE-6033: Add
VMEventListener for DUnit Rules
Posted by "kirklund (GitHub)" <gi...@apache.org>.
[ pull request closed by kirklund ]
[ Full content available at: https://github.com/apache/geode/pull/3161 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund commented on pull request #3161:
GEODE-6033: Add VMEventListener for DUnit Rules
Posted by "kirklund (GitHub)" <gi...@apache.org>.
Updated and added new test that confirms concurrent behavior of registration/notification.
[ Full content available at: https://github.com/apache/geode/pull/3161 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund commented on issue #3161: GEODE-6033: Add
VMEventListener for DUnit Rules
Posted by "kirklund (GitHub)" <gi...@apache.org>.
Build fails on this branch because junitparams dependency needs to be added to added to geode-dunit for new unit test of VMEventNotifier.
[ Full content available at: https://github.com/apache/geode/pull/3161 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] upthewaterspout commented on pull request #3161:
GEODE-6033: Add VMEventListener for DUnit Rules
Posted by "upthewaterspout (GitHub)" <gi...@apache.org>.
Some javadocs would be helpful. For example, when does add get called? If there are already some VMs up and running, how does this listener know about that at the time the VMEventListener is added? Will it's add method be called?
[ Full content available at: https://github.com/apache/geode/pull/3161 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund commented on pull request #3161:
GEODE-6033: Add VMEventListener for DUnit Rules
Posted by "kirklund (GitHub)" <gi...@apache.org>.
Previous push should have added javadocs. I'll check to make sure it made it up to the repo.
[ Full content available at: https://github.com/apache/geode/pull/3161 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund commented on issue #3161: GEODE-6033: Add
VMEventListener for DUnit Rules
Posted by "kirklund (GitHub)" <gi...@apache.org>.
I just realized I should shorten the following:
`VM.getVMEventListenerRegistry().addVMEventListener(vmEventListener);`
To two methods on VM:
```
VM.addVMEventListener(vmEventListener);
VM.removeVMEventListener(vmEventListener);
```
I'll look into doing that tomorrow.
[ Full content available at: https://github.com/apache/geode/pull/3161 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund commented on pull request #3161:
GEODE-6033: Add VMEventListener for DUnit Rules
Posted by "kirklund (GitHub)" <gi...@apache.org>.
Yes, I think it's possible.
[ Full content available at: https://github.com/apache/geode/pull/3161 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org