This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new abe89ea976 Remove and prevent use of terminal deprecations (#4032)
abe89ea976 is described below
commit abe89ea97635dee8c00fc7bcba4647a096984490
Author: Christopher Tubbs <ctubb...@apache.org>
AuthorDate: Wed Dec 6 19:54:33 2023 -0500
Remove and prevent use of terminal deprecations (#4032)
Avoid use of `forRemoval = true` to indicate terminally deprecated
items. Although explicitly communicating that something may be removed
is a nice-to-have, the justification for avoiding it is:
1. terminal deprecations are new in Java, and optional to use;
ordinarily deprecated items can still be subject to removal,
especially for code that was ordinarily deprecated prior to the
introduction of this new forRemoval attribute; so, users cannot rely
on only terminally deprecated items being removed
2. terminal deprecations behave differently than ordinary deprecations
in that the use of terminally deprecated items in deprecated code
still triggers a compiler warning, and the proper use of these and
suppression of these warnings are difficult to use correctly or
reason about (see
https://docs.oracle.com/javase/specs/jls/se11/html/jls-9.html).
One example is the deprecation of an interface method when the
subclass is not also deprecated. This will result in a warning only
if the user directly accesses the method on the implementing class
instead of through the interface. For that reason, it is often
recommended to deprecate subclass methods as well. However, for
terminally deprecated interface methods, the mere existence of a
subclass implementation that overrides the interface method is
enough to trigger a warning, even if it is not used directly (it is
used directly by the subclass to define the override, which is
confusing).
However, the most common problem is just that since terminal
deprecations still trigger a warning, even when used in deprecated
code, you have to add excessive warnings suppressions everywhere,
even in deprecated code, to get a clean build. This can also make
code very ugly, because the warnings appear even in imports, so you
have to use fully-qualified class names instead of import statements
(because you can't suppress warnings in import statements).
3. warnings suppressions of terminally deprecated items are not
implemented properly in various IDEs, making it further difficult to
use (see https://bugs.eclipse.org/bugs/show_bug.cgi?id=565271)
4. Semantic Versioning allows for any previously deprecated item to be
removed and cause a breaking change on a major release. These
breaking changes are possible regardless of whether the deprecation
was marked ordinarily or terminally; so the use of this is entirely
unnecessary, as it does not help the user reliably predict what will
be broken in a future major release
There are several options to address forRemoval:
1. Not enforce its use or non-use
2. Always use it
3. Come up with a means to decide under what circumstances it should be
used and what circumstances it shouldn't be used
4. Never use it
The first option leads to confusion and inconsistency. The second option
leads to problems of misuse, IDE errors, and excessive warnings
suppressions. The third option is complicated, and overall, probably
would be inconsistently applied and hard to enforce. So, this PR resorts
to the fourth option.
---
.../core/client/admin/ActiveCompaction.java | 2 +-
.../org/apache/accumulo/core/conf/Property.java | 28 +++++++++++-----------
.../manager/balancer/TabletStatisticsImpl.java | 2 +-
.../core/spi/balancer/SimpleLoadBalancer.java | 2 +-
.../core/spi/balancer/data/TabletStatistics.java | 2 +-
.../util/compaction/CompactionServicesConfig.java | 4 ++--
.../compaction/CompactionServicesConfigTest.java | 2 +-
pom.xml | 4 ++++
.../accumulo/server/util/ManagerMetadataUtil.java | 2 +-
.../server/util/ManagerMetadataUtilTest.java | 2 +-
.../test/compaction/CompactionRateLimitingIT.java | 2 +-
.../test/functional/AssignLocationModeIT.java | 2 +-
.../test/functional/CompactLocationModeIT.java | 2 +-
.../apache/accumulo/test/shell/ConfigSetIT.java | 2 +-
.../org/apache/accumulo/test/util/SlowOps.java | 2 +-
15 files changed, 32 insertions(+), 28 deletions(-)
diff --git
a/core/src/main/java/org/apache/accumulo/core/client/admin/ActiveCompaction.java
b/core/src/main/java/org/apache/accumulo/core/client/admin/ActiveCompaction.java
index 02bd68bef5..dd02f6ba2f 100644
---
a/core/src/main/java/org/apache/accumulo/core/client/admin/ActiveCompaction.java
+++
b/core/src/main/java/org/apache/accumulo/core/client/admin/ActiveCompaction.java
@@ -57,7 +57,7 @@ public abstract class ActiveCompaction {
* @deprecated Chop compactions no longer occur and it's not expected that
listing compaction
* would ever return this.
*/
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
CHOP,
/**
* idle compaction
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java
b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
index d9c633472b..69d75cd0f1 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
@@ -585,15 +585,15 @@ public enum Property {
"The maximum number of concurrent tablet migrations for a tablet
server.", "1.3.5"),
TSERV_MAJC_DELAY("tserver.compaction.major.delay", "30s",
PropertyType.TIMEDURATION,
"Time a tablet server will sleep between checking which tablets need
compaction.", "1.3.5"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
@ReplacedBy(property = COMPACTION_SERVICE_PREFIX)
TSERV_COMPACTION_SERVICE_PREFIX("tserver.compaction.major.service.", null,
PropertyType.PREFIX,
"Prefix for compaction services.", "2.1.0"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
TSERV_COMPACTION_SERVICE_ROOT_PLANNER("tserver.compaction.major.service.root.planner",
DefaultCompactionPlanner.class.getName(), PropertyType.CLASSNAME,
"Compaction planner for root tablet service.", "2.1.0"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
TSERV_COMPACTION_SERVICE_ROOT_RATE_LIMIT("tserver.compaction.major.service.root.rate.limit",
"0B",
PropertyType.BYTES,
"Maximum number of bytes to read or write per second over all major"
@@ -601,11 +601,11 @@ public enum Property {
+ " been deprecated in anticipation of it being removed in a future
release that"
+ " removes the rate limiting feature.",
"2.1.0"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
TSERV_COMPACTION_SERVICE_ROOT_MAX_OPEN(
"tserver.compaction.major.service.root.planner.opts.maxOpen", "30",
PropertyType.COUNT,
"The maximum number of files a compaction will open.", "2.1.0"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
TSERV_COMPACTION_SERVICE_ROOT_EXECUTORS(
"tserver.compaction.major.service.root.planner.opts.executors",
"[{'name':'small','type':'internal','maxSize':'32M','numThreads':1},{'name':'huge','type':'internal','numThreads':1}]"
@@ -613,11 +613,11 @@ public enum Property {
PropertyType.STRING,
"See {% jlink -f
org.apache.accumulo.core.spi.compaction.DefaultCompactionPlanner %}.",
"2.1.0"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
TSERV_COMPACTION_SERVICE_META_PLANNER("tserver.compaction.major.service.meta.planner",
DefaultCompactionPlanner.class.getName(), PropertyType.CLASSNAME,
"Compaction planner for metadata table.", "2.1.0"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
TSERV_COMPACTION_SERVICE_META_RATE_LIMIT("tserver.compaction.major.service.meta.rate.limit",
"0B",
PropertyType.BYTES,
"Maximum number of bytes to read or write per second over all major"
@@ -625,11 +625,11 @@ public enum Property {
+ " been deprecated in anticipation of it being removed in a future
release that"
+ " removes the rate limiting feature.",
"2.1.0"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
TSERV_COMPACTION_SERVICE_META_MAX_OPEN(
"tserver.compaction.major.service.meta.planner.opts.maxOpen", "30",
PropertyType.COUNT,
"The maximum number of files a compaction will open.", "2.1.0"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
TSERV_COMPACTION_SERVICE_META_EXECUTORS(
"tserver.compaction.major.service.meta.planner.opts.executors",
"[{'name':'small','type':'internal','maxSize':'32M','numThreads':2},{'name':'huge','type':'internal','numThreads':2}]"
@@ -637,11 +637,11 @@ public enum Property {
PropertyType.JSON,
"See {% jlink -f
org.apache.accumulo.core.spi.compaction.DefaultCompactionPlanner %}.",
"2.1.0"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
TSERV_COMPACTION_SERVICE_DEFAULT_PLANNER("tserver.compaction.major.service.default.planner",
DefaultCompactionPlanner.class.getName(), PropertyType.CLASSNAME,
"Planner for default compaction service.", "2.1.0"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
TSERV_COMPACTION_SERVICE_DEFAULT_RATE_LIMIT("tserver.compaction.major.service.default.rate.limit",
"0B", PropertyType.BYTES,
"Maximum number of bytes to read or write per second over all major"
@@ -649,11 +649,11 @@ public enum Property {
+ " been deprecated in anticipation of it being removed in a future
release that"
+ " removes the rate limiting feature.",
"2.1.0"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
TSERV_COMPACTION_SERVICE_DEFAULT_MAX_OPEN(
"tserver.compaction.major.service.default.planner.opts.maxOpen", "10",
PropertyType.COUNT,
"The maximum number of files a compaction will open.", "2.1.0"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
TSERV_COMPACTION_SERVICE_DEFAULT_EXECUTORS(
"tserver.compaction.major.service.default.planner.opts.executors",
"[{'name':'small','type':'internal','maxSize':'32M','numThreads':2},{'name':'medium','type':'internal','maxSize':'128M','numThreads':2},{'name':'large','type':'internal','numThreads':2}]"
@@ -762,7 +762,7 @@ public enum Property {
"The number of threads on each tablet server available to retrieve"
+ " summary data, that is not currently in cache, from RFiles.",
"2.0.0"),
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
TSERV_LAST_LOCATION_MODE("tserver.last.location.mode", "compaction",
PropertyType.LAST_LOCATION_MODE,
"Describes how the system will record the 'last' location for tablets,
which can be used for"
diff --git
a/core/src/main/java/org/apache/accumulo/core/manager/balancer/TabletStatisticsImpl.java
b/core/src/main/java/org/apache/accumulo/core/manager/balancer/TabletStatisticsImpl.java
index c6ff022b36..5d5977bcbe 100644
---
a/core/src/main/java/org/apache/accumulo/core/manager/balancer/TabletStatisticsImpl.java
+++
b/core/src/main/java/org/apache/accumulo/core/manager/balancer/TabletStatisticsImpl.java
@@ -46,7 +46,7 @@ public class TabletStatisticsImpl implements TabletStatistics
{
}
@Override
- @SuppressWarnings("removal")
+ @Deprecated
public long getSplitCreationTime() {
return thriftStats.getSplitCreationTime();
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/spi/balancer/SimpleLoadBalancer.java
b/core/src/main/java/org/apache/accumulo/core/spi/balancer/SimpleLoadBalancer.java
index 2f1b42929e..d42b7d26ff 100644
---
a/core/src/main/java/org/apache/accumulo/core/spi/balancer/SimpleLoadBalancer.java
+++
b/core/src/main/java/org/apache/accumulo/core/spi/balancer/SimpleLoadBalancer.java
@@ -331,7 +331,7 @@ public class SimpleLoadBalancer implements TabletBalancer {
TabletId mostRecentlySplit = null;
long splitTime = 0;
for (Entry<TabletId,TabletStatistics> entry : extents.entrySet()) {
- @SuppressWarnings("removal")
+ @SuppressWarnings("deprecation")
long splitCreationTime = entry.getValue().getSplitCreationTime();
if (splitCreationTime >= splitTime) {
splitTime = splitCreationTime;
diff --git
a/core/src/main/java/org/apache/accumulo/core/spi/balancer/data/TabletStatistics.java
b/core/src/main/java/org/apache/accumulo/core/spi/balancer/data/TabletStatistics.java
index 7e233317f3..793f00cf47 100644
---
a/core/src/main/java/org/apache/accumulo/core/spi/balancer/data/TabletStatistics.java
+++
b/core/src/main/java/org/apache/accumulo/core/spi/balancer/data/TabletStatistics.java
@@ -28,7 +28,7 @@ public interface TabletStatistics extends
Comparable<TabletStatistics> {
long getNumEntries();
- @Deprecated(since = "3.1", forRemoval = true)
+ @Deprecated(since = "3.1")
long getSplitCreationTime();
double getIngestRate();
diff --git
a/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionServicesConfig.java
b/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionServicesConfig.java
index 5cf9d0abc8..21a7719011 100644
---
a/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionServicesConfig.java
+++
b/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionServicesConfig.java
@@ -45,14 +45,14 @@ public class CompactionServicesConfig {
private final Map<String,String> plannerPrefixes = new HashMap<>();
private final Map<String,Long> rateLimits = new HashMap<>();
private final Map<String,Map<String,String>> options = new HashMap<>();
- @SuppressWarnings("removal")
+ @SuppressWarnings("deprecation")
private final Property oldPrefix = Property.TSERV_COMPACTION_SERVICE_PREFIX;
private final Property newPrefix = Property.COMPACTION_SERVICE_PREFIX;
long defaultRateLimit;
public static final CompactionServiceId DEFAULT_SERVICE =
CompactionServiceId.of("default");
- @SuppressWarnings("removal")
+ @SuppressWarnings("deprecation")
private long getDefaultThroughput() {
return ConfigurationTypeHelper
.getMemoryAsBytes(Property.TSERV_COMPACTION_SERVICE_DEFAULT_RATE_LIMIT.getDefaultValue());
diff --git
a/core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionServicesConfigTest.java
b/core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionServicesConfigTest.java
index a5921e33eb..57eb2e330a 100644
---
a/core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionServicesConfigTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionServicesConfigTest.java
@@ -31,7 +31,7 @@ import org.junit.jupiter.api.Test;
public class CompactionServicesConfigTest {
- @SuppressWarnings("removal")
+ @SuppressWarnings("deprecation")
private final Property oldPrefix = Property.TSERV_COMPACTION_SERVICE_PREFIX;
private final Property newPrefix = Property.COMPACTION_SERVICE_PREFIX;
diff --git a/pom.xml b/pom.xml
index 0b772f5130..e49fd960d5 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1042,6 +1042,10 @@
<property name="format" value="\s+$" />
<property name="message" value="Line has trailing
whitespace." />
</module>
+ <module name="RegexpSinglelineJava">
+ <property name="format"
value="[@]Deprecated([^)]*forRemoval[^)]*)" />
+ <property name="message" value="forRemoval should not be
used." />
+ </module>
<module name="RegexpSinglelineJava">
<property name="format" value="[@]see\s+[{][@]link" />
<property name="message" value="Javadoc @see does not need
@link: pick one or the other." />
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/util/ManagerMetadataUtil.java
b/server/base/src/main/java/org/apache/accumulo/server/util/ManagerMetadataUtil.java
index 487055a567..7e6f78c163 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/util/ManagerMetadataUtil.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/util/ManagerMetadataUtil.java
@@ -65,7 +65,7 @@ import com.google.common.base.Preconditions;
public class ManagerMetadataUtil {
private static final Logger log =
LoggerFactory.getLogger(ManagerMetadataUtil.class);
- @SuppressWarnings("removal")
+ @SuppressWarnings("deprecation")
private static final Property DEPRECATED_TSERV_LAST_LOCATION_MODE_PROPERTY =
Property.TSERV_LAST_LOCATION_MODE;
diff --git
a/server/base/src/test/java/org/apache/accumulo/server/util/ManagerMetadataUtilTest.java
b/server/base/src/test/java/org/apache/accumulo/server/util/ManagerMetadataUtilTest.java
index 6be2554548..7eb75207d5 100644
---
a/server/base/src/test/java/org/apache/accumulo/server/util/ManagerMetadataUtilTest.java
+++
b/server/base/src/test/java/org/apache/accumulo/server/util/ManagerMetadataUtilTest.java
@@ -43,7 +43,7 @@ public class ManagerMetadataUtilTest {
@BeforeEach
public void before() {
conf = EasyMock.createMock(AccumuloConfiguration.class);
- @SuppressWarnings("removal")
+ @SuppressWarnings("deprecation")
Property DEPRECATED_TSERV_LAST_LOCATION_MODE_PROPERTY =
Property.TSERV_LAST_LOCATION_MODE;
EasyMock.expect(conf.get(DEPRECATED_TSERV_LAST_LOCATION_MODE_PROPERTY)).andReturn("assignment");
context = EasyMock.createMock(ClientContext.class);
diff --git
a/test/src/main/java/org/apache/accumulo/test/compaction/CompactionRateLimitingIT.java
b/test/src/main/java/org/apache/accumulo/test/compaction/CompactionRateLimitingIT.java
index a7e01466b4..a8dcb5405a 100644
---
a/test/src/main/java/org/apache/accumulo/test/compaction/CompactionRateLimitingIT.java
+++
b/test/src/main/java/org/apache/accumulo/test/compaction/CompactionRateLimitingIT.java
@@ -40,7 +40,7 @@ public class CompactionRateLimitingIT extends
ConfigurableMacBase {
public static final long BYTES_TO_WRITE = 10 * 1024 * 1024;
public static final long RATE = 1 * 1024 * 1024;
- @SuppressWarnings("removal")
+ @SuppressWarnings("deprecation")
protected Property getThroughputProp() {
return Property.TSERV_COMPACTION_SERVICE_DEFAULT_RATE_LIMIT;
}
diff --git
a/test/src/main/java/org/apache/accumulo/test/functional/AssignLocationModeIT.java
b/test/src/main/java/org/apache/accumulo/test/functional/AssignLocationModeIT.java
index 801a7076da..079be28989 100644
---
a/test/src/main/java/org/apache/accumulo/test/functional/AssignLocationModeIT.java
+++
b/test/src/main/java/org/apache/accumulo/test/functional/AssignLocationModeIT.java
@@ -44,7 +44,7 @@ import org.junit.jupiter.api.Test;
public class AssignLocationModeIT extends ConfigurableMacBase {
- @SuppressWarnings("removal")
+ @SuppressWarnings("deprecation")
private static final Property DEPRECATED_TSERV_LAST_LOCATION_MODE_PROPERTY =
Property.TSERV_LAST_LOCATION_MODE;
diff --git
a/test/src/main/java/org/apache/accumulo/test/functional/CompactLocationModeIT.java
b/test/src/main/java/org/apache/accumulo/test/functional/CompactLocationModeIT.java
index c832c6f59e..070a1299c1 100644
---
a/test/src/main/java/org/apache/accumulo/test/functional/CompactLocationModeIT.java
+++
b/test/src/main/java/org/apache/accumulo/test/functional/CompactLocationModeIT.java
@@ -43,7 +43,7 @@ import org.junit.jupiter.api.Test;
public class CompactLocationModeIT extends ConfigurableMacBase {
- @SuppressWarnings("removal")
+ @SuppressWarnings("deprecation")
private static final Property DEPRECATED_TSERV_LAST_LOCATION_MODE_PROPERTY =
Property.TSERV_LAST_LOCATION_MODE;
diff --git a/test/src/main/java/org/apache/accumulo/test/shell/ConfigSetIT.java
b/test/src/main/java/org/apache/accumulo/test/shell/ConfigSetIT.java
index 2c1ae92d7e..71a5e92ecb 100644
--- a/test/src/main/java/org/apache/accumulo/test/shell/ConfigSetIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/shell/ConfigSetIT.java
@@ -49,7 +49,7 @@ public class ConfigSetIT extends SharedMiniClusterBase {
private static final Logger log = LoggerFactory.getLogger(ConfigSetIT.class);
@Test
- @SuppressWarnings("removal")
+ @SuppressWarnings("deprecation")
public void setInvalidJson() throws Exception {
log.debug("Starting setInvalidJson test ------------------");
diff --git a/test/src/main/java/org/apache/accumulo/test/util/SlowOps.java
b/test/src/main/java/org/apache/accumulo/test/util/SlowOps.java
index 63eb0818ec..14c291a053 100644
--- a/test/src/main/java/org/apache/accumulo/test/util/SlowOps.java
+++ b/test/src/main/java/org/apache/accumulo/test/util/SlowOps.java
@@ -78,7 +78,7 @@ public class SlowOps {
createData();
}
- @SuppressWarnings("removal")
+ @SuppressWarnings("deprecation")
public static void setExpectedCompactions(AccumuloClient client, final int
numParallelExpected) {
final int target = numParallelExpected + 1;
try {
