You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/02/28 21:53:27 UTC
[GitHub] [accumulo] jzgithub1 opened a new pull request #1539: Fixes #1525 -
remove lines added in b67d3f9 in VolumeUtil
jzgithub1 opened a new pull request #1539: Fixes #1525 - remove lines added in b67d3f9 in VolumeUtil
URL: https://github.com/apache/accumulo/pull/1539
I bisected the commits in 1.9 and found commit that broke VolumeIT. It was:
7513626cc Optimize Tablet volume replacement (#1505)
----------------------------------------------------------------
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] [accumulo] ctubbsii commented on issue #1539: Fixes #1525 - remove
lines added in b67d3f9 in VolumeUtil
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1539: Fixes #1525 - remove lines added in b67d3f9 in VolumeUtil
URL: https://github.com/apache/accumulo/pull/1539#issuecomment-593581190
> True but that's not what the optimization returned. This change returned the same one passed as a parameter.
Right. My point was that the optimization assumed that the `ret` one that was constructed in the method would have been the same as the parameter by the time the method was finished and `ret` was returned. However, that appears not be the case. It seems there is some difference between the parameter `TabletFiles` and the `TabletFiles ret`. When the test fails, I'm curious what the exact difference is between the two.
----------------------------------------------------------------
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] [accumulo] milleruntime commented on issue #1539: Fixes #1525 -
remove lines added in b67d3f9 in VolumeUtil
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1539: Fixes #1525 - remove lines added in b67d3f9 in VolumeUtil
URL: https://github.com/apache/accumulo/pull/1539#issuecomment-593555900
> > This is odd, because it is passing in master with this change. Comparing the IT across the two versions is rather difficult though since there have been so many changes.
> > I am also not sure why the original change doesn't work. If you look at the code that calls this method [here](https://github.com/apache/accumulo/blob/94844b3ef88bc9e02b5cb5c8eeff81411ac8221d/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java#L344), the `tabletPaths` object should be initialized with all of the entries.
>
> The TabletFiles that is initialized there isn't the one that gets returned from this method. The one returned from this method is initialized at
>
> https://github.com/apache/accumulo/blob/94844b3ef88bc9e02b5cb5c8eeff81411ac8221d/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java#L151
True but that's not what the optimization returned. This change returned the same one passed as a parameter.
----------------------------------------------------------------
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] [accumulo] milleruntime commented on issue #1539: Fixes #1525 -
remove lines added in b67d3f9 in VolumeUtil
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1539: Fixes #1525 - remove lines added in b67d3f9 in VolumeUtil
URL: https://github.com/apache/accumulo/pull/1539#issuecomment-594052732
> For what it's worth, that code was removed in #1389, which makes sense, since that change made the tablet dir no longer store a persistent absolute volume which would be subject to replacements/decommissioning.
Thanks for looking that up, I was very curious what that code was doing and why it was removed.
> (By re-apply, I mean only revert from 1.x, keep the change in 2.x).
Yep, I was thinking the same thing. Now that we solved the case of the strange code chunk disappearance, I feel better about keeping it in 2.x. I will merge this PR into 1.x and then do a no-op merge forward.
----------------------------------------------------------------
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] [accumulo] milleruntime commented on issue #1539: Fixes #1525 -
remove lines added in b67d3f9 in VolumeUtil
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1539: Fixes #1525 - remove lines added in b67d3f9 in VolumeUtil
URL: https://github.com/apache/accumulo/pull/1539#issuecomment-593462147
Here is the 1.9 code that calls this method: https://github.com/apache/accumulo/blob/9c957ab55f28f10dde1321eb613a9ec3e2fa5322/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java#L359
----------------------------------------------------------------
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] [accumulo] milleruntime commented on issue #1539: Fixes #1525 -
remove lines added in b67d3f9 in VolumeUtil
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1539: Fixes #1525 - remove lines added in b67d3f9 in VolumeUtil
URL: https://github.com/apache/accumulo/pull/1539#issuecomment-593555118
> There is specialized code that deals with previously configured volumes [here](https://github.com/apache/accumulo/pull/1539/files#diff-1f6b72391ba6b0e4ae0508ea1ba77793L255), regardless if the replacements property is currently set so this revert will fix it.
This code no longer exists in master. I am not sure why but it explains the difference in behavior between the versions and why the IT fails in 1.9 branch.
----------------------------------------------------------------
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] [accumulo] ctubbsii commented on issue #1539: Fixes #1525 - remove
lines added in b67d3f9 in VolumeUtil
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1539: Fixes #1525 - remove lines added in b67d3f9 in VolumeUtil
URL: https://github.com/apache/accumulo/pull/1539#issuecomment-593637210
> But i think you may have missed my comment earlier since you posted at almost the exact same time.
I did miss that. Thanks. That could explain the difference. If that's the reason for the failure, then I think it might make sense to re-apply the optimization to the master branch only.
For what it's worth, that code was removed in #1389, which makes sense, since that change made the tablet dir no longer store a persistent absolute volume which would be subject to replacements/decommissioning.
----------------------------------------------------------------
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] [accumulo] ctubbsii commented on issue #1539: Fixes #1525 - remove
lines added in b67d3f9 in VolumeUtil
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1539: Fixes #1525 - remove lines added in b67d3f9 in VolumeUtil
URL: https://github.com/apache/accumulo/pull/1539#issuecomment-593522604
> This is odd, because it is passing in master with this change. Comparing the IT across the two versions is rather difficult though since there have been so many changes.
>
> I am also not sure why the original change doesn't work. If you look at the code that calls this method [here](https://github.com/apache/accumulo/blob/94844b3ef88bc9e02b5cb5c8eeff81411ac8221d/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java#L344), the `tabletPaths` object should be initialized with all of the entries.
The TabletFiles that is initialized there isn't the one that gets returned from this method. The one returned from this method is initialized at https://github.com/apache/accumulo/blob/94844b3ef88bc9e02b5cb5c8eeff81411ac8221d/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java#L151
----------------------------------------------------------------
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] [accumulo] ctubbsii commented on issue #1539: Fixes #1525 - remove
lines added in b67d3f9 in VolumeUtil
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1539: Fixes #1525 - remove lines added in b67d3f9 in VolumeUtil
URL: https://github.com/apache/accumulo/pull/1539#issuecomment-594186919
Merged with `-sours` into 2.1. The original change in #1505 was kept in that branch, which was not affected by the breakage in 1.10.
----------------------------------------------------------------
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] [accumulo] ctubbsii commented on issue #1539: Fixes #1525 - remove
lines added in b67d3f9 in VolumeUtil
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1539: Fixes #1525 - remove lines added in b67d3f9 in VolumeUtil
URL: https://github.com/apache/accumulo/pull/1539#issuecomment-593637948
(By re-apply, I mean only revert from 1.x, keep the change in 2.x).
----------------------------------------------------------------
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] [accumulo] milleruntime merged pull request #1539: Fixes #1525 -
remove lines added in b67d3f9 in VolumeUtil
Posted by GitBox <gi...@apache.org>.
milleruntime merged pull request #1539: Fixes #1525 - remove lines added in b67d3f9 in VolumeUtil
URL: https://github.com/apache/accumulo/pull/1539
----------------------------------------------------------------
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] [accumulo] milleruntime commented on issue #1539: Fixes #1525 -
remove lines added in b67d3f9 in VolumeUtil
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1539: Fixes #1525 - remove lines added in b67d3f9 in VolumeUtil
URL: https://github.com/apache/accumulo/pull/1539#issuecomment-593592769
> When the test fails, I'm curious what the exact difference is between the two.
I can do some more debugging because I am too. But i think you may have missed my comment earlier since you posted at almost the exact same time. There is a block of code at the bottom of the method that was removed in master which the optimization prevents from executing: https://github.com/apache/accumulo/blob/1.9/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java#L255
----------------------------------------------------------------
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] [accumulo] milleruntime commented on issue #1539: Fixes #1525 -
remove lines added in b67d3f9 in VolumeUtil
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1539: Fixes #1525 - remove lines added in b67d3f9 in VolumeUtil
URL: https://github.com/apache/accumulo/pull/1539#issuecomment-593461044
This is odd, because it is passing in master with this change. Comparing the IT across the two versions is rather difficult though since there have been so many changes.
I am also not sure why the original change doesn't work. If you look at the code that calls this method [here](https://github.com/apache/accumulo/blob/94844b3ef88bc9e02b5cb5c8eeff81411ac8221d/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java#L344), the ```tabletPaths``` object should be initialized with all of the entries.
----------------------------------------------------------------
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