Re: [PR] Use Duration instead of long+TimeUnit in ReadOnlyTStore.unreserve [accumulo]
cshannon commented on PR #4371: URL: https://github.com/apache/accumulo/pull/4371#issuecomment-2131238795 This change does not merge forward into elasticity because the code for dealing with Fate has drastically changed and trying to resolve conflicts doesn't really work as the code has diverged so much. I am thinking we may want to merge this forward as an empty commit to the elasticity branch using git merge -s and then create a follow on PR to apply these changes to elasticity. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Use Duration instead of long+TimeUnit in ReadOnlyTStore.unreserve [accumulo]
DomGarguilo merged PR #4371: URL: https://github.com/apache/accumulo/pull/4371 -- 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. To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Use Duration instead of long+TimeUnit in ReadOnlyTStore.unreserve [accumulo]
DomGarguilo commented on PR #4371: URL: https://github.com/apache/accumulo/pull/4371#issuecomment-2079797771 > > I've looked through the changes and they look okay - still trying to workout if the test issues that lead to the revert can be duplicated and assigned a reason. > > I also took a quick look at the older commit and could not find what the problem was. I suspect it some time unit conversion issue where the code does something like call sleep with a nanosecond value which basically asking the code sleep forever. However I could not find any issue like that when looking at the change. I am also not sure where the problem was coming from. Do you think its safe to merge this in and open a new ticket if that issue arises again? @EdColeman @keith-turner -- 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. To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Use Duration instead of long+TimeUnit in ReadOnlyTStore.unreserve [accumulo]
keith-turner commented on PR #4371: URL: https://github.com/apache/accumulo/pull/4371#issuecomment-1999737675 > I've looked through the changes and they look okay - still trying to workout if the test issues that lead to the revert can be duplicated and assigned a reason. I also took a quick look at the older commit and could not find what the problem was. I suspect it some time unit conversion issue where the code does something like call sleep with a nanosecond value which basically asking the code sleep forever. However I could not find any issue like that when looking at the change. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Use Duration instead of long+TimeUnit in ReadOnlyTStore.unreserve [accumulo]
EdColeman commented on PR #4371: URL: https://github.com/apache/accumulo/pull/4371#issuecomment-1998103428 I've looked through the changes and they look okay - still trying to workout if the test issues that lead to the revert can be duplicated and assigned a reason. While these changes do not fail, the previous iteration seemed to have a reproducible issue that failed for multiple people. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Use Duration instead of long+TimeUnit in ReadOnlyTStore.unreserve [accumulo]
EdColeman commented on PR #4371: URL: https://github.com/apache/accumulo/pull/4371#issuecomment-1997830067 I confirmed that running -Psunny with these changes passes cleanly. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Use Duration instead of long+TimeUnit in ReadOnlyTStore.unreserve [accumulo]
DomGarguilo opened a new pull request, #4371: URL: https://github.com/apache/accumulo/pull/4371 This re-implements the changes that were originally made in #4358 and then reverted in 23e17129de0350d5496345c78d7fd86080bb1115 since they were causing issues with at least one test hanging ([see details](https://github.com/apache/accumulo/issues/4361#issuecomment-1995475831)). These changes to not cause the same (or any) tests to hang. Partially addressed #4361 -- 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. To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
