You are viewing a plain text version of this content. The canonical link for it is here.
Posted to devnull@infra.apache.org by "nabarunnag (GitHub)" <gi...@apache.org> on 2018/08/24 01:14:27 UTC
[GitHub] [geode] nabarunnag opened pull request #2375: GEODE-5631:
failedBatchRemovalMessageKeys not used after GII
* After GII a flag is set to indicate that failedBatchRemovalMessageKeys has been processed
* If this flag is set, no more entries will be put into failedBatchRemovalMessageKeys.
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [ ] Is your initial contribution a single, squashed commit?
- [ ] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
[ Full content available at: https://github.com/apache/geode/pull/2375 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [geode] dschneider-pivotal commented on pull request #2375:
GEODE-5631: failedBatchRemovalMessageKeys not used after GII
Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
I think these new methods could be made private and have all the code that deals with failedBatchRemoveMessageKeys in this class instead of in subclasses.
For example addToFailedBatchRemovalMessageKeys could do the check isFailedBatchRemovalMessageKeysClearedFlag in this calls instead of having the caller do it.
It is okay with me if this refactoring is done in a future PR.
[ Full content available at: https://github.com/apache/geode/pull/2375 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [geode] jhuynh1 commented on pull request #2375: GEODE-5631:
failedBatchRemovalMessageKeys not used after GII
Posted by "jhuynh1 (GitHub)" <gi...@apache.org>.
Would you be able to move this variable to where we describe all the other variables?
[ Full content available at: https://github.com/apache/geode/pull/2375 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [geode] nabarunnag commented on pull request #2375:
GEODE-5631: failedBatchRemovalMessageKeys not used after GII
Posted by "nabarunnag (GitHub)" <gi...@apache.org>.
@metatype
Going through the documentation about auto-reconnect, when a member disconnects from a distributed system, it shuts down and restarts in a "connecting state".
When it restarts, all the BucketRegionQueues are created again hence setting the flag to false, does the GII and then cleans out any failed batch removal messages that could have arrived during the GII phase and then sets the failedBatchRemovalMessageKeysClearedFlag to true.
[ Full content available at: https://github.com/apache/geode/pull/2375 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [geode] metatype commented on pull request #2375:
GEODE-5631: failedBatchRemovalMessageKeys not used after GII
Posted by "metatype (GitHub)" <gi...@apache.org>.
Will this logic work with auto-reconnect? If a member is on the losing side of a partition it will do GII when it rejoins the cluster, right?
[ Full content available at: https://github.com/apache/geode/pull/2375 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [geode] nabarunnag commented on pull request #2375:
GEODE-5631: failedBatchRemovalMessageKeys not used after GII
Posted by "nabarunnag (GitHub)" <gi...@apache.org>.
@metatype let me go through the auto-reconnect code to figure out how the mechanism works. BRB
[ Full content available at: https://github.com/apache/geode/pull/2375 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [geode] metatype commented on issue #2375: GEODE-5631:
failedBatchRemovalMessageKeys not used after GII
Posted by "metatype (GitHub)" <gi...@apache.org>.
Sounds good, thanks for checking!
> On Aug 23, 2018, at 10:39 PM, Nabarun Nag <no...@github.com> wrote:
>
> @nabarunnag commented on this pull request.
>
> In geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/ParallelQueueRemovalMessage.java:
>
> > @@ -207,8 +207,12 @@ private void destroyKeyFromBucketQueue(AbstractBucketRegionQueue brq, Object key
> }
> // add the key to failedBatchRemovalMessageQueue.
> // This is to handle the last scenario in #49196
> - brq.addToFailedBatchRemovalMessageKeys(key);
> -
> + // But if GII is already completed and FailedBatchRemovalMessageKeys
> + // are already cleared then no keys should be added to it as they will
> + // never be cleared and increase the memory footprint.
> + if (!brq.isFailedBatchRemovalMessageKeysClearedFlag()) {
> @metatype
> Going through the documentation about auto-reconnect, when a member disconnects from a distributed system, it shuts down and restarts in a "connecting state".
>
> When it restarts, all the BucketRegionQueues are created again hence setting the flag to false, does the GII and then cleans out any failed batch removal messages that could have arrived during the GII phase and then sets the failedBatchRemovalMessageKeysClearedFlag to true.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
[ Full content available at: https://github.com/apache/geode/pull/2375 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [geode] nabarunnag closed pull request #2375: GEODE-5631:
failedBatchRemovalMessageKeys not used after GII
Posted by "nabarunnag (GitHub)" <gi...@apache.org>.
[ pull request closed by nabarunnag ]
[ Full content available at: https://github.com/apache/geode/pull/2375 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org