> On March 22, 2016, 6:31 p.m., Barry Oglesby wrote:
> > I'm not sure how this fixes the original bug, but it does fix the
> > BatchRemovalThreads being left running.
> >
> > Can you also change this code to not call getQueues 3 times:
> >
> > From:
> >
> > if (getQueues() != null && !getQueues().isEmpty()) {
> > for (RegionQueue q : getQueues()) {
> > ((SerialGatewaySenderQueue)q).cleanUp();
> > }
> > }
> >
> > To:
> >
> > Set<RegionQueue> queues = getQueues();
> > if (queues != null && !queues.isEmpty()) {
> > for (RegionQueue q : queues) {
> > ((SerialGatewaySenderQueue)q).cleanUp();
> > }
> > }
> >
> > Also, SerialGatewaySenderOperationsDUnitTest validateQueueClosed isn't
> > implemented. Is that intentional?
The original problem was that the thread leaks would eventually cause the
system to run out of memory and not be able to create a native thread. This
caused the sender to fail and the reciever would just wait for events that
would never be arriving.
I'll make the change to getQueues.
The validate queue closed was never implemented and I tried implementing to
verify in this case but by the time that code gets called, the call to
getQueues() would always return null (because eventProcessor is null). I had
to do the verification on the stopSender itself...
I tried to leave the eventProcessor around (like In ParallelGatewaySender case)
but that caused an issue when restarting the sender. It would get into some
code to remove listeners because the eventProcessor is no longer null... it's
very fragile and we probably need to figure out what the actual behavior should
be...
- Jason
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45162/#review124851
-----------------------------------------------------------
On March 22, 2016, 5 p.m., Jason Huynh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45162/
> -----------------------------------------------------------
>
> (Updated March 22, 2016, 5 p.m.)
>
>
> Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan
> Smith, and xiaojian zhou.
>
>
> Bugs: GEODE-911
> https://issues.apache.org/jira/browse/GEODE-911
>
>
> Repository: geode
>
>
> Description
> -------
>
> The stop() method was setting the EventProcessor to null before cleaning up
> the queues. This led to no queues being cleaned up because the method
> getQueues would return null if the eventProcessor was null.
>
> This includes refactoring a few methods in WanTestBase that pretty much did
> the same thing but was duplicated all over.
>
> The Parallel queues do not have this problem because the EventProcessor is
> not set to null "for test purposes." It seems like this should also be set
> to null as well and the tests that rely on it, should be fixed.
>
>
> Diffs
> -----
>
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderQueue.java
> 430409a
>
> geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderImpl.java
> 8f0070f
>
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/WANTestBase.java
> 5da6b5c
>
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/concurrent/ConcurrentSerialGatewaySenderOperationsOffHeapDUnitTest.java
> e24f593
>
> geode-wan/src/test/java/com/gemstone/gemfire/internal/cache/wan/serial/SerialGatewaySenderOperationsDUnitTest.java
> c824eb6
>
> Diff: https://reviews.apache.org/r/45162/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jason Huynh
>
>
