Remove deprecated properties #3106
Conversation
Clean up and remove several deprecated properties for the 3.0.0 release This closes apache#3105
|
I'd like a couple people to take a look to make sure I didn't screw anything up when I was removing deprecated properties. I ran through the sunny tests and they passed but we should also kick off a full IT if it looks good. As I noted in #3105, this is the first pass to remove most of the properties that were easy to clean up. Follow on issues will be done for more in depth changes for things like compaction strategy. |
core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/util/threads/ThreadPools.java
Outdated
Show resolved
Hide resolved
core/src/test/java/org/apache/accumulo/core/conf/AccumuloConfigurationTest.java
Show resolved
Hide resolved
core/src/test/java/org/apache/accumulo/core/conf/AccumuloConfigurationTest.java
Show resolved
Hide resolved
server/base/src/test/java/org/apache/accumulo/server/master/balancer/TableLoadBalancerTest.java
Outdated
Show resolved
Hide resolved
server/manager/src/main/java/org/apache/accumulo/manager/recovery/RecoveryManager.java
Outdated
Show resolved
Hide resolved
is not necessary with no deprecated arguments
The updated test now uses two existing properties that are not deprecated and uses relfection to temporarily mark as deprecated just for testing that resolve() works as the previous properties were removed.
server/base/src/test/java/org/apache/accumulo/server/master/balancer/TableLoadBalancerTest.java
Outdated
Show resolved
Hide resolved
| @SuppressWarnings("deprecation") | ||
| String path = siteConf.get(Property.TRACE_ZK_PATH); | ||
| try { | ||
| zapDirectory(zoo, path, opts); |
There was a problem hiding this comment.
Do we need to remove this as a final step in the upgrade from 2.1 -> 3.0? I didn't look to see if this is already done.
There was a problem hiding this comment.
Do we need to remove this as a final step in the upgrade from 2.1 -> 3.0? I didn't look to see if this is already done.
This should already be done in 2.1. We don't need to keep this for 3.0 upgrade, but we could suggest users remove the /tracer directory themselves, or run the ZooZap util with the property still set (which may be done already on bin/accumulo-cluster stop already.
There was a problem hiding this comment.
@ctubbsii - Is there a good place to log a warning for this? I don't see an Upgrader class yet for 3.0 so is that something we should create in another PR?
There was a problem hiding this comment.
I don't think we need a warning for this. I meant suggest it in the release notes. Accumulo no longer has to care about the existence of this directory. It should have attempted removal on upgrading to 2.1, and if it missed it, it's not a big deal.
server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer1/BulkImport.java
Outdated
Show resolved
Hide resolved
core/src/test/java/org/apache/accumulo/core/conf/AccumuloConfigurationTest.java
Show resolved
Hide resolved
| @SuppressWarnings("deprecation") | ||
| String path = siteConf.get(Property.TRACE_ZK_PATH); | ||
| try { | ||
| zapDirectory(zoo, path, opts); |
There was a problem hiding this comment.
Do we need to remove this as a final step in the upgrade from 2.1 -> 3.0? I didn't look to see if this is already done.
This should already be done in 2.1. We don't need to keep this for 3.0 upgrade, but we could suggest users remove the /tracer directory themselves, or run the ZooZap util with the property still set (which may be done already on bin/accumulo-cluster stop already.
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
Outdated
Show resolved
Hide resolved
|
Ok I updated again with the latest feedback, I think I finally got rid of all the places using resolve where it's no longer necessary. |
|
I updated to remove the deprecated HadoopLogCloser class as well as it was moved to a new location and the deprecated property is removed as part of this PR |
|
@ctubbsii - I forgot this PR was still open as well but I will have to fix the merge conflicts, probably Friday. |
|
Ok I fixed the merge conflicts so it should be ready to go. |
This fixes the test that was broken by apache#3106 and also restores the ability to rename legacy master prefixed properties to manager
test This fixes the test that was broken by apache#3106 by adding back in the master to manager property renamer for just the test. Even though the old master prefixed properties were removed and upgrade isn't needed there are no other renamers that currently exist so adding back in the renamer just for the test will at least verify the functionality still works.
test This fixes the test that was broken by apache#3106 by adding back in the master to manager property renamer for just the test. Even though the old master prefixed properties were removed and upgrade isn't needed there are no other renamers that currently exist so adding back in the renamer just for the test will at least verify the functionality still works.
Clean up and remove several deprecated properties for the 3.0.0 release
This closes #3105