You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2020/01/23 00:59:29 UTC
[GitHub] [phoenix] gjacoby126 opened a new pull request #689: PHOENIX-5677 - Replace System.currentTimeMillis with EnvironmentEdgeM…
gjacoby126 opened a new pull request #689: PHOENIX-5677 - Replace System.currentTimeMillis with EnvironmentEdgeM…
URL: https://github.com/apache/phoenix/pull/689
…anager in non-test code (Addendum to fix IndexScrutinyIT)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] gjacoby126 merged pull request #689: PHOENIX-5677 - Replace System.currentTimeMillis with EnvironmentEdgeM…
Posted by GitBox <gi...@apache.org>.
gjacoby126 merged pull request #689: PHOENIX-5677 - Replace System.currentTimeMillis with EnvironmentEdgeM…
URL: https://github.com/apache/phoenix/pull/689
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] gjacoby126 commented on issue #689: PHOENIX-5677 - Replace System.currentTimeMillis with EnvironmentEdgeM…
Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on issue #689: PHOENIX-5677 - Replace System.currentTimeMillis with EnvironmentEdgeM…
URL: https://github.com/apache/phoenix/pull/689#issuecomment-577480448
So, the errors came from 2 sources.
1. IndexScrutinyMapperForTest is really test code, inheriting from EnvironmentEdge, but is in production code, so 5677 converted it over to use EnvironmentEdgeManger incorrectly, causing infinite recursion.
2. PartialIndexRebuilderIT has a strange test, testIndexActiveIfRegionMovesWhileRebuilding, which was apparently passing before these changes but is failing now. It's multi-threaded, delicately pieced together using both Thread sleeps AND the edge manager. More importantly, it seems to be asserting something incorrect: that the partial rebuilder should succeed and make a disabled (old-style) index ACTIVE even if a region gets unassigned (and not re-assigned) in the middle of the rebuild run. But then the MR job should fail and the index shouldn't be reactivated. So since I don't understand the purpose of the test, I've gone ahead and removed it. If someone sees a valid use case to test there, please let me know and I'll try again to make it work.
These would have been caught pre-commit, but the Jenkins job was broken for a few days OOMing on each run.
@kadirozde @lhofhansl
----------------------------------------------------------------
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
With regards,
Apache Git Services