(accumulo) branch elasticity updated: avoid unnecessary metadata reads in client tablet cache (#4432)

[kturner]

This is an automated email from the ASF dual-hosted git repository.

kturner pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/elasticity by this push:
 new f586e7f9c9 avoid unnecessary metadata reads in client tablet cache 
(#4432)
f586e7f9c9 is described below

commit f586e7f9c9a82c0f01cba1ac279a7d583c7be3c4
Author: Keith Turner 
AuthorDate: Fri Apr 5 12:05:21 2024 -0400

avoid unnecessary metadata reads in client tablet cache (#4432)

For ondemand tablets the client tablet cache caches tablets w/o a
location.  There was a bug fixed in #4280 where the cache would do a
metadata table lookup for each mutation when tablets had no location.
The fix in #4280 only partially fixed the problem, after that change
more metadata lookups than needed were still being done.

Also there was a bug with the batchscanner that #4280 did not address.
Before this change when tablets had no location, the batch scanner would
do a metadata lookup for each range passed to the batch scanner (well
the client tablet cache would these metadata lookups on behalf of the
batch scanner).  For example before this change if the batch scanner was
given 10K ranges that all fell in a single tablet w/o a location, it
would do 10K metadata lookups.  After this change for that situation it
will do a single metadata lookup.

This change minimizes the metadata lookups done by the batch writer and
batch scanner.  The fix is to make sure that cached entries populated by
looking up one range or mutation are used by subsequent range or
mutations lookups, even if there is no location present. This is done by
always reusing cache entries that were created after work started on a
batch of mutations or ranges.  Cache entries w/o a location that existed
before work started on a batch are ignored.  By reusing cache entries
created after starting work on a batch we minimize metadata lookups.

A test was also added to ensure the client tablet cache does not do
excessive metadata table lookups.  If this test had existed, it would
have caught the problem.
---
 .../core/clientImpl/ClientTabletCache.java |   8 +-
 .../core/clientImpl/ClientTabletCacheImpl.java |  66 ++
 .../core/clientImpl/ClientTabletCacheImplTest.java | 137 +
 3 files changed, 181 insertions(+), 30 deletions(-)

diff --git 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCache.java 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCache.java
index 2564fb0063..a31ca2418b 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCache.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCache.java
@@ -20,7 +20,6 @@ package org.apache.accumulo.core.clientImpl;
 
 import static com.google.common.base.Preconditions.checkArgument;
 
-import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -46,6 +45,7 @@ import org.apache.accumulo.core.singletons.SingletonManager;
 import org.apache.accumulo.core.singletons.SingletonService;
 import org.apache.accumulo.core.util.Interner;
 import org.apache.accumulo.core.util.UtilWaitThread;
+import org.apache.accumulo.core.util.time.NanoTime;
 import org.apache.hadoop.io.Text;
 
 import com.google.common.base.Preconditions;
@@ -311,7 +311,7 @@ public abstract class ClientTabletCache {
 private final TabletAvailability availability;
 private final boolean hostingRequested;
 
-private final Long creationTime = System.nanoTime();
+private final NanoTime creationTime = NanoTime.now();
 
 public CachedTablet(KeyExtent tablet_extent, String tablet_location, 
String session,
 TabletAvailability availability, boolean hostingRequested) {
@@ -392,8 +392,8 @@ public abstract class ClientTabletCache {
   return this.availability;
 }
 
-public Duration getAge() {
-  return Duration.ofNanos(System.nanoTime() - creationTime);
+public NanoTime getCreationTime() {
+  return creationTime;
 }
 
 public boolean wasHostingRequested() {
diff --git 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java
 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java
index 10fb3aa21e..eb5225 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImpl.java
@@ -60,6 +60,7 @@ import org.apache.accumulo.core.trace.TraceUtil;
 import org.apache.accumulo.core.util.OpTimer;
 import org.apache.accumulo.core.util.Pair;
 import org.apache.accumulo.core.util.TextUtil;
+import org.apache.accumulo.core.util.time.NanoTime;
 import org.apache.hadoop.io.Text;
 import 

(accumulo) branch elasticity updated: moves ample filters out of public API (#4431)

[kturner]

This is an automated email from the ASF dual-hosted git repository.

kturner pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/elasticity by this push:
 new 0386506c1d moves ample filters out of public API (#4431)
0386506c1d is described below

commit 0386506c1d91a5edfcc524df17ee472992adc46d
Author: Keith Turner 
AuthorDate: Fri Apr 5 12:05:38 2024 -0400

moves ample filters out of public API (#4431)

Ample filters were in a package that is in the public API. Since Ample
itself is not in the public API, these filters should also not be in the
public API.  This commit moves the filters to another packages that is
not in the public API
---
 .../org/apache/accumulo/core/metadata/schema/TabletsMetadata.java   | 2 +-
 .../{iterators/user => metadata/schema/filters}/GcWalsFilter.java   | 5 +
 .../user => metadata/schema/filters}/HasCurrentFilter.java  | 2 +-
 .../schema/filters}/HasExternalCompactionsFilter.java   | 2 +-
 .../user => metadata/schema/filters}/TabletMetadataFilter.java  | 3 ++-
 .../java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java   | 2 +-
 .../manager/compaction/coordinator/CompactionCoordinator.java   | 2 +-
 .../manager/compaction/coordinator/DeadCompactionDetector.java  | 2 +-
 .../org/apache/accumulo/monitor/rest/tables/TablesResource.java | 2 +-
 .../apache/accumulo/test/functional/AmpleConditionalWriterIT.java   | 6 +++---
 .../apache/accumulo/test/functional/TestTabletMetadataFilter.java   | 2 +-
 11 files changed, 14 insertions(+), 16 deletions(-)

diff --git 
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java
 
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java
index 14b8181e78..48a1160513 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletsMetadata.java
@@ -63,7 +63,6 @@ import org.apache.accumulo.core.data.TableId;
 import org.apache.accumulo.core.dataImpl.KeyExtent;
 import org.apache.accumulo.core.fate.zookeeper.ZooCache;
 import org.apache.accumulo.core.fate.zookeeper.ZooReader;
-import org.apache.accumulo.core.iterators.user.TabletMetadataFilter;
 import org.apache.accumulo.core.iterators.user.WholeRowIterator;
 import org.apache.accumulo.core.metadata.AccumuloTable;
 import org.apache.accumulo.core.metadata.RootTable;
@@ -85,6 +84,7 @@ import 
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.Sp
 import 
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SuspendLocationColumn;
 import 
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.UserCompactionRequestedColumnFamily;
 import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType;
+import org.apache.accumulo.core.metadata.schema.filters.TabletMetadataFilter;
 import org.apache.accumulo.core.security.Authorizations;
 import org.apache.accumulo.core.util.ColumnFQ;
 import org.apache.hadoop.io.Text;
diff --git 
a/core/src/main/java/org/apache/accumulo/core/iterators/user/GcWalsFilter.java 
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/filters/GcWalsFilter.java
similarity index 94%
rename from 
core/src/main/java/org/apache/accumulo/core/iterators/user/GcWalsFilter.java
rename to 
core/src/main/java/org/apache/accumulo/core/metadata/schema/filters/GcWalsFilter.java
index a30302ce59..4e1aca1a06 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/iterators/user/GcWalsFilter.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/filters/GcWalsFilter.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.accumulo.core.iterators.user;
+package org.apache.accumulo.core.metadata.schema.filters;
 
 import java.io.IOException;
 import java.util.Arrays;
@@ -41,9 +41,6 @@ import com.google.common.collect.Sets;
  * A filter used by the Accumulo GC to find tablets that either have walogs or 
are assigned to a
  * dead tablet server.
  */
-
-// ELASTICITY_TODO Move TabletMetadataFilter and its subclasses out of public 
API. It use internal
-// types that are not user facing.
 public class GcWalsFilter extends TabletMetadataFilter {
 
   private Map options = null;
diff --git 
a/core/src/main/java/org/apache/accumulo/core/iterators/user/HasCurrentFilter.java
 
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/filters/HasCurrentFilter.java
similarity index 96%
rename from 
core/src/main/java/org/apache/accumulo/core/iterators/user/HasCurrentFilter.java
rename to 
core/src/main/java/org/apache/accumulo/core/metadata/schema/filters/HasCurrentFilter.java
index ca58922306..0d00b59837 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/iterators/user/HasCurrentFilter.java
+++ 

(accumulo) branch elasticity updated (637dd0fd3f -> eea8ce48e2)

[dlmarion]

This is an automated email from the ASF dual-hosted git repository.

dlmarion pushed a change to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git


from 637dd0fd3f Removed elasticity comment to do in ExternalCompaction_2_IT 
(#4416)
 add 9d4d68b2a3 Changed return variable for 
ExternalCompactionUtil.getCompactorAddrs (#4419)
 new eea8ce48e2 Merge branch 'main' into elasticity

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/accumulo/core/clientImpl/InstanceOperationsImpl.java   | 2 +-
 .../accumulo/core/util/compaction/ExternalCompactionUtil.java | 7 ---
 .../src/main/java/org/apache/accumulo/monitor/Monitor.java| 2 +-
 .../monitor/rest/compactions/external/CoordinatorInfo.java| 4 ++--
 .../monitor/rest/compactions/external/ExternalCompactionInfo.java | 8 
 .../org/apache/accumulo/test/functional/MemoryStarvedMajCIT.java  | 6 --
 6 files changed, 16 insertions(+), 13 deletions(-)

(accumulo) 01/01: Merge branch 'main' into elasticity

[dlmarion]

This is an automated email from the ASF dual-hosted git repository.

dlmarion pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git

commit eea8ce48e2d5d7bd5c6b69e05c6824f697d8de0a
Merge: 637dd0fd3f 9d4d68b2a3
Author: Dave Marion 
AuthorDate: Fri Apr 5 15:09:17 2024 +

Merge branch 'main' into elasticity

 .../apache/accumulo/core/clientImpl/InstanceOperationsImpl.java   | 2 +-
 .../accumulo/core/util/compaction/ExternalCompactionUtil.java | 7 ---
 .../src/main/java/org/apache/accumulo/monitor/Monitor.java| 2 +-
 .../monitor/rest/compactions/external/CoordinatorInfo.java| 4 ++--
 .../monitor/rest/compactions/external/ExternalCompactionInfo.java | 8 
 .../org/apache/accumulo/test/functional/MemoryStarvedMajCIT.java  | 6 --
 6 files changed, 16 insertions(+), 13 deletions(-)

diff --cc 
core/src/main/java/org/apache/accumulo/core/util/compaction/ExternalCompactionUtil.java
index 86181247a8,0046af7dc6..48895192bd
--- 
a/core/src/main/java/org/apache/accumulo/core/util/compaction/ExternalCompactionUtil.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/util/compaction/ExternalCompactionUtil.java
@@@ -105,26 -106,26 +106,26 @@@ public class ExternalCompactionUtil 
}
  
/**
 -   * @return map of queue names to compactor addresses
 +   * @return map of group names to compactor addresses
 */
-   public static Map> getCompactorAddrs(ClientContext 
context) {
+   public static Map> getCompactorAddrs(ClientContext 
context) {
  try {
-   final Map> groupsAndAddresses = new 
HashMap<>();
 -  final Map> queuesAndAddresses = new HashMap<>();
 -  final String compactorQueuesPath = context.getZooKeeperRoot() + 
Constants.ZCOMPACTORS;
++  final Map> groupsAndAddresses = new HashMap<>();
 +  final String compactorGroupsPath = context.getZooKeeperRoot() + 
Constants.ZCOMPACTORS;
ZooReader zooReader = context.getZooReader();
 -  List queues = zooReader.getChildren(compactorQueuesPath);
 -  for (String queue : queues) {
 -queuesAndAddresses.putIfAbsent(queue, new HashSet<>());
 +  List groups = zooReader.getChildren(compactorGroupsPath);
 +  for (String group : groups) {
  try {
 -  List compactors = zooReader.getChildren(compactorQueuesPath 
+ "/" + queue);
 +  List compactors = zooReader.getChildren(compactorGroupsPath 
+ "/" + group);
for (String compactor : compactors) {
  // compactor is the address, we are checking to see if there is a 
child node which
  // represents the compactor's lock as a check that it's alive.
  List children =
 -zooReader.getChildren(compactorQueuesPath + "/" + queue + "/" 
+ compactor);
 +zooReader.getChildren(compactorGroupsPath + "/" + group + "/" 
+ compactor);
  if (!children.isEmpty()) {
LOG.trace("Found live compactor {} ", compactor);
-   groupsAndAddresses.putIfAbsent(group, new ArrayList<>());
 -  
queuesAndAddresses.get(queue).add(HostAndPort.fromString(compactor));
++  groupsAndAddresses.putIfAbsent(group, new HashSet<>());
 +  
groupsAndAddresses.get(group).add(HostAndPort.fromString(compactor));
  }
}
  } catch (NoNodeException e) {
diff --cc 
server/monitor/src/main/java/org/apache/accumulo/monitor/rest/compactions/external/CoordinatorInfo.java
index d17ce19f18,8724f758bb..eccda4569e
--- 
a/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/compactions/external/CoordinatorInfo.java
+++ 
b/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/compactions/external/CoordinatorInfo.java
@@@ -33,9 -33,9 +33,9 @@@ public class CoordinatorInfo 
  
public CoordinatorInfo(Optional serverOpt, 
ExternalCompactionInfo ecInfo) {
  server = serverOpt.map(HostAndPort::toString).orElse("none");
 -var queueToCompactors = ecInfo.getCompactors();
 -numQueues = queueToCompactors.size();
 -numCompactors = 
queueToCompactors.values().stream().mapToInt(Set::size).sum();
 +var groupToCompactors = ecInfo.getCompactors();
 +numQueues = groupToCompactors.size();
- numCompactors = 
groupToCompactors.values().stream().mapToInt(List::size).sum();
++numCompactors = 
groupToCompactors.values().stream().mapToInt(Set::size).sum();
  lastContact = System.currentTimeMillis() - ecInfo.getFetchedTimeMillis();
}
  }
diff --cc 
test/src/main/java/org/apache/accumulo/test/functional/MemoryStarvedMajCIT.java
index 03f7442c91,771d74d588..637a71eca8
--- 
a/test/src/main/java/org/apache/accumulo/test/functional/MemoryStarvedMajCIT.java
+++ 
b/test/src/main/java/org/apache/accumulo/test/functional/MemoryStarvedMajCIT.java
@@@ -22,10 -22,9 +22,12 @@@ import static org.apache.accumulo.test.
  import static org.junit.jupiter.api.Assertions.assertEquals;
  import static 

(accumulo) branch main updated: Changed return variable for ExternalCompactionUtil.getCompactorAddrs (#4419)

[dlmarion]

This is an automated email from the ASF dual-hosted git repository.

dlmarion 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 9d4d68b2a3 Changed return variable for 
ExternalCompactionUtil.getCompactorAddrs (#4419)
9d4d68b2a3 is described below

commit 9d4d68b2a373df2c36763a5ed55675a5f8d127a3
Author: Dave Marion 
AuthorDate: Fri Apr 5 10:55:12 2024 -0400

Changed return variable for ExternalCompactionUtil.getCompactorAddrs (#4419)
---
 .../apache/accumulo/core/clientImpl/InstanceOperationsImpl.java   | 2 +-
 .../accumulo/core/util/compaction/ExternalCompactionUtil.java | 7 ---
 .../org/apache/accumulo/coordinator/CompactionCoordinator.java| 8 
 .../src/main/java/org/apache/accumulo/monitor/Monitor.java| 2 +-
 .../monitor/rest/compactions/external/CoordinatorInfo.java| 4 ++--
 .../monitor/rest/compactions/external/ExternalCompactionInfo.java | 8 
 6 files changed, 16 insertions(+), 15 deletions(-)

diff --git 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
index 74bd2ece59..215f7c6214 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
@@ -303,7 +303,7 @@ public class InstanceOperationsImpl implements 
InstanceOperations {
   public List getActiveCompactions()
   throws AccumuloException, AccumuloSecurityException {
 
-Map> compactors = 
ExternalCompactionUtil.getCompactorAddrs(context);
+Map> compactors = 
ExternalCompactionUtil.getCompactorAddrs(context);
 List tservers = getTabletServers();
 
 int numThreads = Math.max(4, Math.min((tservers.size() + 
compactors.size()) / 10, 256));
diff --git 
a/core/src/main/java/org/apache/accumulo/core/util/compaction/ExternalCompactionUtil.java
 
b/core/src/main/java/org/apache/accumulo/core/util/compaction/ExternalCompactionUtil.java
index 35c358b7ed..0046af7dc6 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/util/compaction/ExternalCompactionUtil.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/util/compaction/ExternalCompactionUtil.java
@@ -25,6 +25,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
@@ -107,14 +108,14 @@ public class ExternalCompactionUtil {
   /**
* @return map of queue names to compactor addresses
*/
-  public static Map> getCompactorAddrs(ClientContext 
context) {
+  public static Map> getCompactorAddrs(ClientContext 
context) {
 try {
-  final Map> queuesAndAddresses = new HashMap<>();
+  final Map> queuesAndAddresses = new HashMap<>();
   final String compactorQueuesPath = context.getZooKeeperRoot() + 
Constants.ZCOMPACTORS;
   ZooReader zooReader = context.getZooReader();
   List queues = zooReader.getChildren(compactorQueuesPath);
   for (String queue : queues) {
-queuesAndAddresses.putIfAbsent(queue, new ArrayList<>());
+queuesAndAddresses.putIfAbsent(queue, new HashSet<>());
 try {
   List compactors = zooReader.getChildren(compactorQueuesPath 
+ "/" + queue);
   for (String compactor : compactors) {
diff --git 
a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
 
b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
index 7358ce4416..c86a93350c 100644
--- 
a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
+++ 
b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
@@ -304,7 +304,7 @@ public class CompactionCoordinator extends AbstractServer
 
   long now = System.currentTimeMillis();
 
-  Map> idleCompactors = getIdleCompactors();
+  Map> idleCompactors = getIdleCompactors();
   TIME_COMPACTOR_LAST_CHECKED.forEach((queue, lastCheckTime) -> {
 if ((now - lastCheckTime) > getMissingCompactorWarningTime()
 && QUEUE_SUMMARIES.isCompactionsQueued(queue) && 
idleCompactors.containsKey(queue)) {
@@ -325,16 +325,16 @@ public class CompactionCoordinator extends AbstractServer
 LOG.info("Shutting down");
   }
 
-  private Map> getIdleCompactors() {
+  private Map> getIdleCompactors() {
 
-Map> allCompactors =
+Map> allCompactors =
 ExternalCompactionUtil.getCompactorAddrs(getContext());
 
 Set emptyQueues = new HashSet<>();
 
 // Remove all of the compactors that are running a compaction
 RUNNING_CACHE.values().forEach(rc -> {
- 

(accumulo) branch elasticity updated: Removed elasticity comment to do in ExternalCompaction_2_IT (#4416)

[dlmarion]

This is an automated email from the ASF dual-hosted git repository.

dlmarion pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/elasticity by this push:
 new 637dd0fd3f Removed elasticity comment to do in ExternalCompaction_2_IT 
(#4416)
637dd0fd3f is described below

commit 637dd0fd3f4ce9e408b93db59816e1b2b07b6a9e
Author: Dave Marion 
AuthorDate: Fri Apr 5 10:11:25 2024 -0400

Removed elasticity comment to do in ExternalCompaction_2_IT (#4416)

The comment said that the operation id needed to be set when
deleting the tablets. This is now done in ReserveTablets.isReady.


Co-authored-by: Keith Turner 
---
 .../org/apache/accumulo/test/compaction/ExternalCompaction_2_IT.java  | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git 
a/test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompaction_2_IT.java
 
b/test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompaction_2_IT.java
index 937cb03037..4ca3794ca9 100644
--- 
a/test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompaction_2_IT.java
+++ 
b/test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompaction_2_IT.java
@@ -238,9 +238,7 @@ public class ExternalCompaction_2_IT extends 
SharedMiniClusterBase {
   confirmCompactionCompleted(getCluster().getServerContext(), ecids,
   TCompactionState.CANCELLED);
 
-  // ELASTICITY_TODO make delete table fate op get operation ids before 
deleting
-  // there should be no metadata for the table, check to see if the 
compaction wrote anything
-  // after table delete
+  // Ensure compaction did not write anything to metadata table after 
delete table
   try (var scanner = 
client.createScanner(AccumuloTable.METADATA.tableName())) {
 scanner.setRange(MetadataSchema.TabletsSection.getRange(tid));
 assertEquals(0, scanner.stream().count());

(accumulo) branch main updated: avoids acquiring recovery lock when tablet has no wals (#4429)

[kturner]

This is an automated email from the ASF dual-hosted git repository.

kturner 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 b912506d43 avoids acquiring recovery lock when tablet has no wals 
(#4429)
b912506d43 is described below

commit b912506d43fbac9477aa3efb9ea81d39ce4d1a5c
Author: Keith Turner 
AuthorDate: Fri Apr 5 13:01:07 2024 -0400

avoids acquiring recovery lock when tablet has no wals (#4429)

Tablet servers have a lock for recovery that has the purpose of
preventing concurrent recovery of tablets using too much memory.
This lock is acquired for tablets that have no walogs and will therefore
do no recovery.  This commit changes the code to only obtain the lock
when there are walogs.

Noticed this while reviewing #4404 and while researching found
[ACCUMULO-1543](https://issues.apache.org/jira/browse/ACCUMULO-1543)
which is an existing issue describing this change.
---
 .../org/apache/accumulo/tserver/AssignmentHandler.java   |  6 +-
 .../java/org/apache/accumulo/tserver/TabletServer.java   | 16 
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java
 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java
index 8b4f117a21..552d9f40a9 100644
--- 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java
+++ 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java
@@ -156,9 +156,7 @@ class AssignmentHandler implements Runnable {
 Tablet tablet = null;
 boolean successful = false;
 
-try {
-  server.acquireRecoveryMemory(extent);
-
+try (var recoveryMemory = server.acquireRecoveryMemory(tabletMetadata)) {
   TabletResourceManager trm = 
server.resourceManager.createTabletResourceManager(extent,
   server.getTableConfiguration(extent));
   TabletData data = new TabletData(tabletMetadata);
@@ -205,8 +203,6 @@ class AssignmentHandler implements Runnable {
   TableId tableId = extent.tableId();
   ProblemReports.getInstance(server.getContext()).report(new 
ProblemReport(tableId, TABLET_LOAD,
   extent.getUUID().toString(), server.getClientAddressString(), e));
-} finally {
-  server.releaseRecoveryMemory(extent);
 }
 
 if (successful) {
diff --git 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
index b9c9a39359..678b1294c5 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
@@ -91,6 +91,7 @@ import org.apache.accumulo.core.manager.thrift.TableInfo;
 import org.apache.accumulo.core.manager.thrift.TabletServerStatus;
 import org.apache.accumulo.core.metadata.AccumuloTable;
 import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
 import org.apache.accumulo.core.metadata.schema.TabletsMetadata;
 import org.apache.accumulo.core.metrics.MetricsUtil;
 import org.apache.accumulo.core.rpc.ThriftUtil;
@@ -516,15 +517,14 @@ public class TabletServer extends AbstractServer 
implements TabletHostingServer
 managerMessages.addLast(m);
   }
 
-  void acquireRecoveryMemory(KeyExtent extent) {
-if (!extent.isMeta()) {
-  recoveryLock.lock();
-}
-  }
+  private static final AutoCloseable NOOP_CLOSEABLE = () -> {};
 
-  void releaseRecoveryMemory(KeyExtent extent) {
-if (!extent.isMeta()) {
-  recoveryLock.unlock();
+  AutoCloseable acquireRecoveryMemory(TabletMetadata tabletMetadata) {
+if (tabletMetadata.getExtent().isMeta() || 
tabletMetadata.getLogs().isEmpty()) {
+  return NOOP_CLOSEABLE;
+} else {
+  recoveryLock.lock();
+  return recoveryLock::unlock;
 }
   }
 

(accumulo) branch elasticity updated: Removed TODO in TabletRefresher (#4417)

[dlmarion]

This is an automated email from the ASF dual-hosted git repository.

dlmarion pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/elasticity by this push:
 new 995f318459 Removed TODO in TabletRefresher (#4417)
995f318459 is described below

commit 995f318459efe6e1c3b29cf8fc9c0a27302f04dc
Author: Dave Marion 
AuthorDate: Fri Apr 5 16:13:40 2024 -0400

Removed TODO in TabletRefresher (#4417)

Other exceptions which could be more serious than TException would
be raised as an ExecutionException when get() is called on the Future
and would result in a RuntimeException being raised from
TabletRefresher.refreshTablets. If a TException is thrown, then
the refresh for all of the Tablets will be retried unless the
TabletServer is no longer online.
---
 .../org/apache/accumulo/manager/tableOps/bulkVer2/TabletRefresher.java | 3 ---
 1 file changed, 3 deletions(-)

diff --git 
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/TabletRefresher.java
 
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/TabletRefresher.java
index a3d341a12b..cb963e583a 100644
--- 
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/TabletRefresher.java
+++ 
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/TabletRefresher.java
@@ -171,9 +171,6 @@ public class TabletRefresher {
 } catch (TException ex) {
   log.debug("rpc failed server: " + location + ", " + logId + " " + 
ex.getMessage(), ex);
 
-  // ELASTICITY_TODO are there any other exceptions we should catch in 
this method and check if
-  // the tserver is till alive?
-
   // something went wrong w/ RPC return all extents as unrefreshed
   return refreshes;
 } finally {

(accumulo) branch elasticity updated (0386506c1d -> cb097aee5f)

[kturner]

This is an automated email from the ASF dual-hosted git repository.

kturner pushed a change to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git


from 0386506c1d moves ample filters out of public API (#4431)
 add b912506d43 avoids acquiring recovery lock when tablet has no wals 
(#4429)
 new cb097aee5f Merge branch 'main' into elasticity

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../org/apache/accumulo/tserver/AssignmentHandler.java   |  6 +-
 .../java/org/apache/accumulo/tserver/TabletServer.java   | 16 
 2 files changed, 9 insertions(+), 13 deletions(-)

(accumulo) 01/01: Merge branch 'main' into elasticity

[kturner]

This is an automated email from the ASF dual-hosted git repository.

kturner pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git

commit cb097aee5f36fb91f04c09db084595ef624cd2c8
Merge: 0386506c1d b912506d43
Author: Keith Turner 
AuthorDate: Fri Apr 5 15:19:05 2024 -0400

Merge branch 'main' into elasticity

 .../org/apache/accumulo/tserver/AssignmentHandler.java   |  6 +-
 .../java/org/apache/accumulo/tserver/TabletServer.java   | 16 
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --cc 
server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java
index cef355cbf1,552d9f40a9..ec56611a03
--- 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java
+++ 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java
@@@ -131,13 -156,12 +131,11 @@@ class AssignmentHandler implements Runn
  Tablet tablet = null;
  boolean successful = false;
  
- try {
-   server.acquireRecoveryMemory(extent);
- 
+ try (var recoveryMemory = server.acquireRecoveryMemory(tabletMetadata)) {
TabletResourceManager trm = 
server.resourceManager.createTabletResourceManager(extent,
server.getTableConfiguration(extent));
 -  TabletData data = new TabletData(tabletMetadata);
  
 -  tablet = new Tablet(server, extent, trm, data);
 +  tablet = new Tablet(server, extent, trm, tabletMetadata);
// If a minor compaction starts after a tablet opens, this indicates a 
log recovery
// occurred. This recovered data must be minor compacted.
// There are three reasons to wait for this minor compaction to finish 
before placing the
diff --cc 
server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
index 63fcbd4c5b,678b1294c5..026c904ca2
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
@@@ -87,7 -91,8 +87,8 @@@ import org.apache.accumulo.core.manager
  import org.apache.accumulo.core.manager.thrift.TabletServerStatus;
  import org.apache.accumulo.core.metadata.AccumuloTable;
  import org.apache.accumulo.core.metadata.TServerInstance;
 +import org.apache.accumulo.core.metadata.schema.Ample.TabletsMutator;
+ import org.apache.accumulo.core.metadata.schema.TabletMetadata;
 -import org.apache.accumulo.core.metadata.schema.TabletsMetadata;
  import org.apache.accumulo.core.metrics.MetricsUtil;
  import org.apache.accumulo.core.rpc.ThriftUtil;
  import org.apache.accumulo.core.rpc.clients.ThriftClientTypes;

(accumulo) branch elasticity updated: Removed FileUtil.cleanupIndexOp to resolve TODO, related changes (#4385)

[dlmarion]

This is an automated email from the ASF dual-hosted git repository.

dlmarion pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/elasticity by this push:
 new b6769788c0 Removed FileUtil.cleanupIndexOp to resolve TODO, related 
changes (#4385)
b6769788c0 is described below

commit b6769788c054407bc1ee269758684a86b8ca6e10
Author: Dave Marion 
AuthorDate: Fri Apr 5 16:12:51 2024 -0400

Removed FileUtil.cleanupIndexOp to resolve TODO, related changes (#4385)

The existing TODO in FileUtil was to determine if the split code
in elasticity was missing something. The cleanupIndexOp method was
called in earlier versions, but is no longer called in elasticity.
I determined that the SplitUtils.IndexIterable.close method was a
likely replacement for the cleanupIndexOp method. I removed this
method and FileUtilTest as it was only testing this method. The
remaining method in FileUtil is only called from Splitter, so I
moved the method, related code, and associated test. I also fixed
up references that were broken due to the code move.
---
 .../org/apache/accumulo/server/util/FileUtil.java  | 135 
 .../apache/accumulo/server/util/FileUtilTest.java  | 176 -
 .../apache/accumulo/manager/split/Splitter.java|  81 +-
 .../manager/tableOps/split/UpdateTablets.java  |   6 +-
 .../manager/upgrade/SplitRecovery12to13.java   |   6 +-
 .../manager/tableOps/split}/FileInfoTest.java  |   4 +-
 .../manager/tableOps/split/UpdateTabletsTest.java  |   5 +-
 7 files changed, 88 insertions(+), 325 deletions(-)

diff --git 
a/server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java 
b/server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java
deleted file mode 100644
index 78a541ca6e..00
--- a/server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java
+++ /dev/null
@@ -1,135 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   https://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.accumulo.server.util;
-
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Set;
-
-import org.apache.accumulo.core.file.FileOperations;
-import org.apache.accumulo.core.file.FileSKVIterator;
-import org.apache.accumulo.core.metadata.TabletFile;
-import org.apache.accumulo.server.ServerContext;
-import org.apache.accumulo.server.conf.TableConfiguration;
-import org.apache.accumulo.server.fs.VolumeManager;
-import org.apache.hadoop.fs.FileSystem;
-import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.io.Text;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-public class FileUtil {
-
-  public static class FileInfo {
-final Text firstRow;
-final Text lastRow;
-
-public FileInfo(Text firstRow, Text lastRow) {
-  this.firstRow = firstRow;
-  this.lastRow = lastRow;
-}
-
-public Text getFirstRow() {
-  return firstRow;
-}
-
-public Text getLastRow() {
-  return lastRow;
-}
-  }
-
-  private static final Logger log = LoggerFactory.getLogger(FileUtil.class);
-
-  // ELASTICITY_TODO this is only used by test. Determine what the test are 
doing and if some
-  // functionality is missing in the new split code.
-  protected static void cleanupIndexOp(Path tmpDir, VolumeManager fs,
-  ArrayList readers) throws IOException {
-// close all of the index sequence files
-for (FileSKVIterator r : readers) {
-  try {
-if (r != null) {
-  r.close();
-}
-  } catch (IOException e) {
-// okay, try to close the rest anyway
-log.error("{}", e.getMessage(), e);
-  }
-}
-
-if (tmpDir != null) {
-  FileSystem actualFs = fs.getFileSystemByPath(tmpDir);
-  if (actualFs.exists(tmpDir)) {
-fs.deleteRecursively(tmpDir);
-return;
-  }
-
-  log.error("Did not delete tmp dir because it wasn't a tmp dir {}", 
tmpDir);
-}
-  }
-
-  public static  Map 
tryToGetFirstAndLastRows(
-  ServerContext context, TableConfiguration tableConf, Set dataFiles) {
-
-HashMap 

(accumulo) branch elasticity updated: Existing logging in ManagerClientServiceHandler is sufficient, removed todo (#4433)

[dlmarion]

This is an automated email from the ASF dual-hosted git repository.

dlmarion pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/elasticity by this push:
 new 77e95f95b9 Existing logging in ManagerClientServiceHandler is 
sufficient, removed todo (#4433)
77e95f95b9 is described below

commit 77e95f95b9b3b76c42f5b619937dbcb58553f029
Author: Dave Marion 
AuthorDate: Fri Apr 5 16:28:31 2024 -0400

Existing logging in ManagerClientServiceHandler is sufficient, removed todo 
(#4433)
---
 .../java/org/apache/accumulo/manager/ManagerClientServiceHandler.java| 1 -
 1 file changed, 1 deletion(-)

diff --git 
a/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java
 
b/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java
index 9a1fdaf478..dd14024db6 100644
--- 
a/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java
+++ 
b/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java
@@ -648,7 +648,6 @@ public class ManagerClientServiceHandler implements 
ManagerClientService.Iface {
   inProgress.forEach(hostingRequestInProgress::remove);
 }
 
-// ELASTICITY_TODO pass ranges of individual tablets
 manager.getEventCoordinator().event(success, "Tablet hosting requested for 
%d tablets in %s",
 success.size(), tableId);
   }