You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by upthewaterspout <gi...@git.apache.org> on 2017/03/16 17:21:33 UTC

[GitHub] geode pull request #427: GEODE-2674: Changing the lucene listener to get fro...

GitHub user upthewaterspout opened a pull request:

    https://github.com/apache/geode/pull/427

    GEODE-2674: Changing the lucene listener to get from the region

    Rather than use the value that is in the queue, use the latest value
    from the region to update the lucene index.
    
    This ensures that even if the queue contains spurious events due to
    retries or other issues, we put the correct value in the index.
    
    This also potentially saves memory and disk space for the queue, because
    the queue does not need to hold the value for the entry.
    
    @boglesby @gesterzhou @jhuynh1 @ladyVader @nabarunnag 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/upthewaterspout/incubator-geode feature/GEODE-2674

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/geode/pull/427.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #427
    
----
commit 9f9439ab0ec6c2a0221e78aa5825e9a0759bac80
Author: Dan Smith <up...@apache.org>
Date:   2017-03-15T20:23:20Z

    GEODE-2674: Changing the lucene listener to fetch the value from the region
    
    Rather than use the value that is in the queue, use the latest value
    from the region to update the lucene index.
    
    This ensures that even if the queue contains spurious events due to
    retries or other issues, we put the correct value in the index.
    
    This also potentially saves memory and disk space for the queue, because
    the queue does not need to hold the value for the entry.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #427: GEODE-2674: Changing the lucene listener to get fro...

Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on a diff in the pull request:

    https://github.com/apache/geode/pull/427#discussion_r106484913
  
    --- Diff: geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventSubstitutionFilter.java ---
    @@ -0,0 +1,37 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    + * agreements. See the NOTICE file distributed with this work for additional information regarding
    + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance with the License. You may obtain a
    + * copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software distributed under the License
    + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
    + * or implied. See the License for the specific language governing permissions and limitations under
    + * the License.
    + */
    +
    +package org.apache.geode.cache.lucene.internal;
    +
    +import org.apache.geode.cache.EntryEvent;
    +import org.apache.geode.cache.wan.GatewayEventSubstitutionFilter;
    +import org.apache.geode.internal.cache.Token;
    +
    +/**
    + * A substitution filter which throws away the value of the entry and replaces it with an INVALID
    --- End diff --
    
    You are right. I tried Token.NOT_AVAILABLE and then Token.INVALID, but that made the queue unhappy. I'll change the comment. I thought I was using a static string? Is there something that would be better to use here?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #427: GEODE-2674: Changing the lucene listener to get fro...

Posted by jhuynh1 <gi...@git.apache.org>.
Github user jhuynh1 commented on a diff in the pull request:

    https://github.com/apache/geode/pull/427#discussion_r106482198
  
    --- Diff: geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventSubstitutionFilter.java ---
    @@ -0,0 +1,37 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    + * agreements. See the NOTICE file distributed with this work for additional information regarding
    + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance with the License. You may obtain a
    + * copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software distributed under the License
    + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
    + * or implied. See the License for the specific language governing permissions and limitations under
    + * the License.
    + */
    +
    +package org.apache.geode.cache.lucene.internal;
    +
    +import org.apache.geode.cache.EntryEvent;
    +import org.apache.geode.cache.wan.GatewayEventSubstitutionFilter;
    +import org.apache.geode.internal.cache.Token;
    +
    +/**
    + * A substitution filter which throws away the value of the entry and replaces it with an INVALID
    --- End diff --
    
    incorrect comment?  we are not returning an INVALID token but rather an empty string?  Should we use a static string for this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #427: GEODE-2674: Changing the lucene listener to get fro...

Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on a diff in the pull request:

    https://github.com/apache/geode/pull/427#discussion_r106504069
  
    --- Diff: geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventSubstitutionFilter.java ---
    @@ -0,0 +1,37 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    + * agreements. See the NOTICE file distributed with this work for additional information regarding
    + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance with the License. You may obtain a
    + * copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software distributed under the License
    + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
    + * or implied. See the License for the specific language governing permissions and limitations under
    + * the License.
    + */
    +
    +package org.apache.geode.cache.lucene.internal;
    +
    +import org.apache.geode.cache.EntryEvent;
    +import org.apache.geode.cache.wan.GatewayEventSubstitutionFilter;
    +import org.apache.geode.internal.cache.Token;
    +
    +/**
    + * A substitution filter which throws away the value of the entry and replaces it with an INVALID
    --- End diff --
    
    Hmm, I was thinking null would just mean remove the event or don't substitute the value. But maybe the gateway actually handles that appropriately. Let me test that.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #427: GEODE-2674: Changing the lucene listener to get fro...

Posted by gesterzhou <gi...@git.apache.org>.
Github user gesterzhou commented on a diff in the pull request:

    https://github.com/apache/geode/pull/427#discussion_r106503358
  
    --- Diff: geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventSubstitutionFilter.java ---
    @@ -0,0 +1,37 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    + * agreements. See the NOTICE file distributed with this work for additional information regarding
    + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance with the License. You may obtain a
    + * copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software distributed under the License
    + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
    + * or implied. See the License for the specific language governing permissions and limitations under
    + * the License.
    + */
    +
    +package org.apache.geode.cache.lucene.internal;
    +
    +import org.apache.geode.cache.EntryEvent;
    +import org.apache.geode.cache.wan.GatewayEventSubstitutionFilter;
    +import org.apache.geode.internal.cache.Token;
    +
    +/**
    + * A substitution filter which throws away the value of the entry and replaces it with an INVALID
    --- End diff --
    
    Is "null" ok? Otherwise, it's so far so good to use "".  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode issue #427: GEODE-2674: Changing the lucene listener to get from the r...

Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on the issue:

    https://github.com/apache/geode/pull/427
  
    Changes are merged


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #427: GEODE-2674: Changing the lucene listener to get fro...

Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout closed the pull request at:

    https://github.com/apache/geode/pull/427


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---