You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by vitogav <gi...@git.apache.org> on 2015/08/04 20:08:57 UTC

[GitHub] incubator-geode pull request: Fix for GEODE-109

GitHub user vitogav opened a pull request:

    https://github.com/apache/incubator-geode/pull/13

    Fix for GEODE-109

    1) To fix this floating meta data problem for the redis list implementation, this meta data has been moved into the list region itself
    2) Sorted set query failures are fixed by using fully qualified names and also depends on GEODE-146
    3) Concurrent region creation/destruction logic completely changed to use dedicated locks for synchronization, fixed distributed deadlock
    4) Added integration tests
    5) Add javadocs where necessary

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

    $ git pull https://github.com/vitogav/incubator-geode GEODE-109

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

    https://github.com/apache/incubator-geode/pull/13.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 #13
    
----
commit b7acb4ba70cddaa3f18f8ac074d5041693f75510
Author: Vito Gavrilov <vg...@pivotal.io>
Date:   2015-07-28T16:40:42Z

    Fix for GEODE-109
    
    1) To fix this floating meta data problem for the redis list implementation, this meta data has been moved into the list region itself
    2) Sorted set query failures are fixed by using fully qualified names and also depends on GEODE-146
    3) Concurrent region creation/destruction logic completely changed to use dedicated locks for synchronization, fixed distributed deadlock
    4) Added integration tests
    5) Add javadocs where necessary

----


---
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] incubator-geode pull request: Fix for GEODE-109

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

    https://github.com/apache/incubator-geode/pull/13#discussion_r36257113
  
    --- Diff: gemfire-core/src/main/java/com/gemstone/gemfire/internal/redis/ExecutionHandlerContext.java ---
    @@ -179,31 +203,21 @@ public void executeCommand(ChannelHandlerContext ctx, Command command) throws Ex
        * 
        * @param exec Executor to use
        * @param command Command to execute
    -   * @param cache Cache instance
    -   * @param client Client data associated with client
    -   * @param n Recursive max depth of retries
        * @throws Exception Throws exception if exception is from within execution and not to be handled
        */
    -  private void executeWithoutTransaction(final Executor exec, Command command, int n) throws Exception {
    -    try {
    -      exec.executeCommand(command, this);
    -    } catch (RegionDestroyedException e) {
    -      if (n > 0)
    -        executeWithoutTransaction(exec, command, n - 1);
    -      else
    -        throw e;
    +  private void executeWithoutTransaction(final Executor exec, Command command) throws Exception {
    +    for (int i = 0; i < MAXIMUM_NUM_RETRIES; i++) {
    +      try {
    +        exec.executeCommand(command, this);
    +        return;
    +      } catch (Exception e) {
    +        if (e instanceof RegionDestroyedException || e.getCause() instanceof QueryInvocationTargetException)
    +          Thread.sleep(WAIT_REGION_DSTRYD_MILLIS);
    +      }
         }
    +    throw new Exception("Could not execute command");
    --- End diff --
    
    Can you please re-throw the underlying exception 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] incubator-geode pull request: Fix for GEODE-109

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

    https://github.com/apache/incubator-geode/pull/13#discussion_r36256817
  
    --- Diff: gemfire-core/src/main/java/com/gemstone/gemfire/internal/redis/Command.java ---
    @@ -116,7 +106,8 @@ public String getStringKey() {
           if (this.key == null) {
             this.bytes = new ByteArrayWrapper(this.commandElems.get(1));
             this.key = this.bytes.toString();
    -      }
    +      } else if (this.key == null)
    --- End diff --
    
    The if and else if condition here looks the same.


---
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] incubator-geode pull request: Fix for GEODE-109

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

    https://github.com/apache/incubator-geode/pull/13#discussion_r36256895
  
    --- Diff: gemfire-core/src/main/java/com/gemstone/gemfire/internal/redis/DoubleWrapper.java ---
    @@ -37,12 +43,19 @@ public int compareTo(Object arg0) {
           other = ((DoubleWrapper) arg0).score;
         } else if (arg0 instanceof String) {
           String arg = (String) arg0;
    +      try {
    --- End diff --
    
    Please check to see if comparing to String is required.


---
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] incubator-geode pull request: Fix for GEODE-109

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

    https://github.com/apache/incubator-geode/pull/13


---
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.
---