You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/06/10 00:45:08 UTC

[GitHub] [geode] onichols-pivotal opened a new pull request #5230: Revert "GEODE-8234: redis functions now implement InternalFunction"

onichols-pivotal opened a new pull request #5230:
URL: https://github.com/apache/geode/pull/5230


   Reverts apache/geode#5229


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] onichols-pivotal edited a comment on pull request #5230: Revert "GEODE-8234: redis functions now implement InternalFunction"

Posted by GitBox <gi...@apache.org>.
onichols-pivotal edited a comment on pull request #5230:
URL: https://github.com/apache/geode/pull/5230#issuecomment-641658570


   @jdeppe-pivotal Since #5229, UpgradeTest has failed 5/6 times in ClientServerMiscBCDUnitTest. testPingWrongServer, both in the develop pipeline and the PR pipeline (even your own PR #5228 is now hitting this).  Examples:
   
   1. [UpgradeTestOpenJDK8 #254](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK8/builds/254)
   2. [UpgradeTestOpenJDK11 #250](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK11/builds/250)
   3. [UpgradeTestOpenJDK11 #251](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK11/builds/251)
   4. [UpgradeTestOpenJDK11 #8214](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/UpgradeTestOpenJDK11/builds/8214)
   5. [UpgradeTestOpenJDK11 #8216](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/UpgradeTestOpenJDK11/builds/8216)
   
   It did pass in:
   1. [UpgradeTestOpenJDK8 #255](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK8/builds/255) 
   2. [this revert PR](https://github.com/apache/geode/pull/5230)
   This rules out the possibility of an environmental issue, so this PR has already served some useful purpose even with a -1 blocking it from being merged.
   
   The ones that did fail have various [n] suffix for the failing test, which along with the 1 out of 6 that passed, suggests some degree of indeterminism.
   
   If the failure had nothing to do with #5229, perhaps the Upgrade tests are sensitive to test ordering?
   
   https://issues.apache.org/jira/browse/GEODE-8236 has been filed for this issue, although it looks like it has been seen before in
   https://issues.apache.org/jira/browse/GEODE-8176
   
   On a recent [dev list discussion](https://lists.apache.org/x/thread.html/rcccca408873c92df789ddf7f2f871f9251498d4b902b9019da7200eb@%3Cdev.geode.apache.org%3E) many in the community favored revert-first-then-figure-it-out, since the alternative of just leaving the pipeline and all PRs broken isn't great either. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] onichols-pivotal edited a comment on pull request #5230: Revert "GEODE-8234: redis functions now implement InternalFunction"

Posted by GitBox <gi...@apache.org>.
onichols-pivotal edited a comment on pull request #5230:
URL: https://github.com/apache/geode/pull/5230#issuecomment-641658570


   @jdeppe-pivotal Since this commit, Upgrade test has consistently failed in ClientServerMiscBCDUnitTest. testPingWrongServer, both in the develop pipeline and the PR pipeline (even your own PR #5228 is now hitting this).  Examples:
   
   1. [UpgradeTestOpenJDK8 #254](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK8/builds/254)
   2. [UpgradeTestOpenJDK11 #250](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK11/builds/250)
   3. [UpgradeTestOpenJDK11 #251](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK11/builds/251)
   4. [UpgradeTestOpenJDK11 #8214](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/UpgradeTestOpenJDK11/builds/8214)
   5. [UpgradeTestOpenJDK11 #8216](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/UpgradeTestOpenJDK11/builds/8216)
   
   It did pass in:
   1. [UpgradeTestOpenJDK8 #255](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK8/builds/255) 
   2. [this revert PR](https://github.com/apache/geode/pull/5230)
   This rules out the possibility of an environmental issue, so this PR has already served some useful purpose even with a -1 blocking it from being merged.  The ones that did fail have various [n] suffix for the failing test, which along with the 1 out of 6 that passed, suggests some degree of indeterminism.
   
   If the failure had nothing to do with this commit, perhaps the Upgrade tests are sensitive to test ordering?
   
   https://issues.apache.org/jira/browse/GEODE-8236 has been filed for this issue, although it looks like it has been seen before in https://issues.apache.org/jira/browse/GEODE-8176
   
   On a recent [dev list discussion](https://lists.apache.org/x/thread.html/rcccca408873c92df789ddf7f2f871f9251498d4b902b9019da7200eb@%3Cdev.geode.apache.org%3E) many in the community favored revert-first-then-figure-it-out, since the alternative of just leaving the pipeline and all PRs broken isn't great either. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] onichols-pivotal closed pull request #5230: Revert "GEODE-8234: redis functions now implement InternalFunction"

Posted by GitBox <gi...@apache.org>.
onichols-pivotal closed pull request #5230:
URL: https://github.com/apache/geode/pull/5230


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] onichols-pivotal edited a comment on pull request #5230: Revert "GEODE-8234: redis functions now implement InternalFunction"

Posted by GitBox <gi...@apache.org>.
onichols-pivotal edited a comment on pull request #5230:
URL: https://github.com/apache/geode/pull/5230#issuecomment-641658570


   @jdeppe-pivotal Since this commit, Upgrade test has consistently failed in ClientServerMiscBCDUnitTest. testPingWrongServer, both in the develop pipeline and the PR pipeline (even your own PR #5228 is now hitting this).  Examples:
   
   1. [UpgradeTestOpenJDK8 #254](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK8/builds/254)
   2. [UpgradeTestOpenJDK11 #250](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK11/builds/250)
   3. [UpgradeTestOpenJDK8 #255](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK8/builds/255)
   4. [UpgradeTestOpenJDK11 #251](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK11/builds/251)
   5. [UpgradeTestOpenJDK11 #8214](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/UpgradeTestOpenJDK11/builds/8214)
   6. [UpgradeTestOpenJDK11 #8216](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/UpgradeTestOpenJDK11/builds/8216)
   
   The only PR it has not failed in since the problem began is [this revert PR](https://github.com/apache/geode/pull/5230), which rules out the possibility of an environmental issue.  So this PR has already served some useful purpose even with a -1 blocking it from being merged.
   
   If the failure had nothing to do with this commit, perhaps the Upgrade tests are sensitive to test ordering?
   
   https://issues.apache.org/jira/browse/GEODE-8236 has been filed for this issue, although it looks like it has been seen before in https://issues.apache.org/jira/browse/GEODE-8176
   
   On a recent [dev list discussion](https://lists.apache.org/x/thread.html/rcccca408873c92df789ddf7f2f871f9251498d4b902b9019da7200eb@%3Cdev.geode.apache.org%3E) many in the community favored revert-first-then-figure-it-out, since the alternative of just leaving the pipeline and all PRs broken isn't great either. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] onichols-pivotal edited a comment on pull request #5230: Revert "GEODE-8234: redis functions now implement InternalFunction"

Posted by GitBox <gi...@apache.org>.
onichols-pivotal edited a comment on pull request #5230:
URL: https://github.com/apache/geode/pull/5230#issuecomment-641658570


   @jdeppe-pivotal Since this commit, Upgrade test has failed 5/6 times in ClientServerMiscBCDUnitTest. testPingWrongServer, both in the develop pipeline and the PR pipeline (even your own PR #5228 is now hitting this).  Examples:
   
   1. [UpgradeTestOpenJDK8 #254](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK8/builds/254)
   2. [UpgradeTestOpenJDK11 #250](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK11/builds/250)
   3. [UpgradeTestOpenJDK11 #251](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK11/builds/251)
   4. [UpgradeTestOpenJDK11 #8214](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/UpgradeTestOpenJDK11/builds/8214)
   5. [UpgradeTestOpenJDK11 #8216](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/UpgradeTestOpenJDK11/builds/8216)
   
   It did pass in:
   1. [UpgradeTestOpenJDK8 #255](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK8/builds/255) 
   2. [this revert PR](https://github.com/apache/geode/pull/5230)
   This rules out the possibility of an environmental issue, so this PR has already served some useful purpose even with a -1 blocking it from being merged.
   
   The ones that did fail have various [n] suffix for the failing test, which along with the 1 out of 6 that passed, suggests some degree of indeterminism.
   
   If the failure had nothing to do with this commit, perhaps the Upgrade tests are sensitive to test ordering?
   
   https://issues.apache.org/jira/browse/GEODE-8236 has been filed for this issue, although it looks like it has been seen before in https://issues.apache.org/jira/browse/GEODE-8176
   
   On a recent [dev list discussion](https://lists.apache.org/x/thread.html/rcccca408873c92df789ddf7f2f871f9251498d4b902b9019da7200eb@%3Cdev.geode.apache.org%3E) many in the community favored revert-first-then-figure-it-out, since the alternative of just leaving the pipeline and all PRs broken isn't great either. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] onichols-pivotal commented on pull request #5230: Revert "GEODE-8234: redis functions now implement InternalFunction"

Posted by GitBox <gi...@apache.org>.
onichols-pivotal commented on pull request #5230:
URL: https://github.com/apache/geode/pull/5230#issuecomment-641658570


   This commit broke https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK8/builds/254 if you can fix it that would be great too, but I believe our policy is revert-then-fix when you break the pipeline...


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] onichols-pivotal edited a comment on pull request #5230: Revert "GEODE-8234: redis functions now implement InternalFunction"

Posted by GitBox <gi...@apache.org>.
onichols-pivotal edited a comment on pull request #5230:
URL: https://github.com/apache/geode/pull/5230#issuecomment-641658570


   @jdeppe-pivotal Since #5229, UpgradeTest has failed 5/6 times in ClientServerMiscBCDUnitTest. testPingWrongServer, both in the develop pipeline and the PR pipeline (even your own PR #5228 is now hitting this).  Examples:
   
   1. [UpgradeTestOpenJDK8 #254](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK8/builds/254)
   2. [UpgradeTestOpenJDK11 #250](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK11/builds/250)
   3. [UpgradeTestOpenJDK11 #251](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK11/builds/251)
   4. [UpgradeTestOpenJDK11 #8214](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/UpgradeTestOpenJDK11/builds/8214)
   5. [UpgradeTestOpenJDK11 #8216](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/UpgradeTestOpenJDK11/builds/8216)
   
   It did pass in:
   1. [UpgradeTestOpenJDK8 #255](https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/UpgradeTestOpenJDK8/builds/255) 
   2. [this revert PR](https://github.com/apache/geode/pull/5230)
   The ones that did fail have various [n] suffix for the failing test, which along with the 1 out of 6 that passed, suggests some degree of indeterminism or flakiness, but it does seem suspicious that it suddenly went from failing rarely to almost every time.
   
   If the failure had nothing to do with #5229, perhaps the Upgrade tests are sensitive to test ordering?
   
   https://issues.apache.org/jira/browse/GEODE-8236 has been filed for this issue, although it looks like it has been seen before in
   https://issues.apache.org/jira/browse/GEODE-8176
   
   On a recent [dev list discussion](https://lists.apache.org/x/thread.html/rcccca408873c92df789ddf7f2f871f9251498d4b902b9019da7200eb@%3Cdev.geode.apache.org%3E) many in the community favored revert-first-then-figure-it-out, since the alternative of just leaving the pipeline and all PRs broken isn't great either. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] dschneider-pivotal commented on pull request #5230: Revert "GEODE-8234: redis functions now implement InternalFunction"

Posted by GitBox <gi...@apache.org>.
dschneider-pivotal commented on pull request #5230:
URL: https://github.com/apache/geode/pull/5230#issuecomment-641725070


   Perhaps it would be better to disable this flaky test until someone works on it. But normally we just look at failures and find open tickets saying that they fail intermittently and ignore that failure. I'm not sure why that did not happen in this case. A ticket already existed but did not seem to help.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] onichols-pivotal commented on pull request #5230: Revert "GEODE-8234: redis functions now implement InternalFunction"

Posted by GitBox <gi...@apache.org>.
onichols-pivotal commented on pull request #5230:
URL: https://github.com/apache/geode/pull/5230#issuecomment-641727580


   sounds like the consensus is not to revert


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org