You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "keith-turner (via GitHub)" <gi...@apache.org> on 2023/07/11 22:06:17 UTC
[GitHub] [accumulo] keith-turner opened a new issue, #3593: Deprecate InstanceOperations.getActiveComactions(String tserver)
keith-turner opened a new issue, #3593:
URL: https://github.com/apache/accumulo/issues/3593
While working on #3592 noticed [this method](https://github.com/apache/accumulo/blob/d4846d407e5b28482394e2c0baa16932ae35e086/core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java#L212) that list the active compactions on a tsever. Given this will always be the empty set now, should probably deprecate this method in 3.x so it can be remove in 4.0.
--
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-unsubscribe@accumulo.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Deprecate InstanceOperations.getActiveComactions(String tserver) [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on issue #3593:
URL: https://github.com/apache/accumulo/issues/3593#issuecomment-1755379223
@keith-turner - what are you saying, that we should close this issue and close the associated PR?
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Deprecate InstanceOperations.getActiveComactions(String tserver) [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on issue #3593:
URL: https://github.com/apache/accumulo/issues/3593#issuecomment-1756368577
> @keith-turner - what are you saying, that we should close this issue and close the associated PR?
The initial premise of this issue was that `InstanceOperations.getActiveComactions(String tserver)` would no longer return anything because tablet server no longer did major compactions. However I did not realize that the method also returned minor compactions running on a tserver until attempting to run a test that checked for that. So I was commenting on the reason for opening the issue being wrong.
Even though the initial premise was wrong I think exploring this area did reveal the following problems.
* No way to list compactors
* InstanceOperations.getActiveComactions(String tserver) does not currently accept a compactor address (and if it did there is no way for users to get them). It could be modified to accept a compactor address.
The PR addressed some of these more general problems in addition to addressing the specific problem. I think there is still value in what the PR was doing. Also, I can't remember if it was this PR or discussion elsewhere but I seem to remember discussion about being able to retrieve resource groups associated with servers.
May need a new issue that describes these problems.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] keith-turner commented on issue #3593: Deprecate InstanceOperations.getActiveComactions(String tserver)
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on issue #3593:
URL: https://github.com/apache/accumulo/issues/3593#issuecomment-1636101124
If we had used more specific types in the API than string in the past then this would be easier to handle. For example if the API had looked like.
```java
interface InstanceOperations {
Collection<TabletServerAddress> getTabletServers();
Collection<ActiveCompactions> getActiveCompactions(TabletServerAddress tsa).
}
```
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Deprecate InstanceOperations.getActiveComactions(String tserver) [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on issue #3593:
URL: https://github.com/apache/accumulo/issues/3593#issuecomment-2315312298
> Even though the initial premise was wrong I think exploring this area did reveal the following problems.
> * No way to list compactors
> * InstanceOperations.getActiveComactions(String tserver) does not currently accept a compactor address (and if it did there is no way for users to get them). It could be modified to accept a compactor address.
Created #4845 to address these issues.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion commented on issue #3593: Deprecate InstanceOperations.getActiveComactions(String tserver)
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on issue #3593:
URL: https://github.com/apache/accumulo/issues/3593#issuecomment-1635959619
So, we could just change the parameter from `tserver` to `server` and update the javadoc. If we remove this method, then there is no way to get the compactions running on a Compactor. Do we need to deprecate this and create a new method for this type of change? I'm thinking not.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] keith-turner commented on issue #3593: Deprecate InstanceOperations.getActiveComactions(String tserver)
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on issue #3593:
URL: https://github.com/apache/accumulo/issues/3593#issuecomment-1636094288
> Do we want to cause users of this to break intentionally so that they have to evaluate their code and use a different API method?
One reason to break it is that code like the following would keep compiling, but would no longer do anything. Not all usages of getActiveCompactions(String) have to be tightly coupled to InstanceOperations.getTabletServers() but its likely some are.
```java
void someUserMethod(AccumuloClient client, Consumer<ActiveCompaction> acc){
var io = client.instanceOperations();
for(String tserver : io.getTabletServers()){
if(match(tserver)) {
acc.accept(io.getActiveCompactions(tserver));
}
}
```
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Deprecate InstanceOperations.getActiveComactions(String tserver) [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion closed issue #3593: Deprecate InstanceOperations.getActiveComactions(String tserver)
URL: https://github.com/apache/accumulo/issues/3593
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion commented on issue #3593: Deprecate InstanceOperations.getActiveComactions(String tserver)
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on issue #3593:
URL: https://github.com/apache/accumulo/issues/3593#issuecomment-1636008428
#3613 just fixes the existing method in the elasticity branch to use the compactor instead of the tserver. Do we want to cause users of this to break intentionally so that they have to evaluate their code and use a different API method?
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Deprecate InstanceOperations.getActiveComactions(String tserver) [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on issue #3593:
URL: https://github.com/apache/accumulo/issues/3593#issuecomment-2318201410
#4845 has been merged. I think that addresses the issues in this PR (deprecation and the original issue are OBE due to later comments).
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Deprecate InstanceOperations.getActiveComactions(String tserver) [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on issue #3593:
URL: https://github.com/apache/accumulo/issues/3593#issuecomment-1751474003
The premise of this issue is wrong, see #3797 and #3827
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org