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 d5f9611 Separate native map loading from native map code (#2425)
d5f9611 is described below
commit d5f9611efbcffc883271b77d0512e8c0102c1079
Author: Christopher Tubbs <ctubb...@apache.org>
AuthorDate: Fri Jan 21 18:34:43 2022 -0500
Separate native map loading from native map code (#2425)
* Create a separate NativeMapLoader and relocate into that new class the
code to load the native libraries that was previously in NativeMap
alongside the actual code for handling native maps
* Remove unnecessarily redundant `isLoaded()` checks, since the behavior
is now to `System.exit(1)` if the native code is configured to be
used, but cannot be loaded. There is no longer a question of whether
they are loaded. They are always loaded if configured to be, or else
the tserver will die on startup.
* Simplify and organize the loader code
* Make the loader code work better with tests by providing a method
specifically for testing (for NativeMapIT, and InMemoryMapIT,
specifically, because they need to load native maps inside the test
thread, and not on a tserver)
* Update ConfigurableMacBase so it will ensure the native maps are built
before Mini is started, so the tests don't hang because the native
maps aren't built (only applies to ConfigurableMacBase tests... other
tests may need something similar)
---
.../org/apache/accumulo/tserver/InMemoryMap.java | 2 +-
.../org/apache/accumulo/tserver/NativeMap.java | 119 +----------------
.../tserver/TabletServerResourceManager.java | 7 +-
.../accumulo/tserver/memory/NativeMapLoader.java | 141 +++++++++++++++++++++
.../org/apache/accumulo/test/InMemoryMapIT.java | 10 +-
.../test/functional/ConfigurableMacBase.java | 13 +-
.../accumulo/test/functional/NativeMapIT.java | 5 +-
7 files changed, 164 insertions(+), 133 deletions(-)
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java
index ea6bf82..87f3725 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java
@@ -156,7 +156,7 @@ public class InMemoryMap {
}
private static SimpleMap newMap(boolean useNativeMap) {
- if (useNativeMap && NativeMap.isLoaded()) {
+ if (useNativeMap) {
try {
return new NativeMapWrapper();
} catch (Exception t) {
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java
index c257bbc..5d4d9fa 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java
@@ -18,10 +18,8 @@
*/
package org.apache.accumulo.tserver;
-import java.io.File;
import java.lang.ref.Cleaner.Cleanable;
import java.util.AbstractMap.SimpleImmutableEntry;
-import java.util.ArrayList;
import java.util.Collection;
import java.util.ConcurrentModificationException;
import java.util.HashSet;
@@ -37,7 +35,6 @@ import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.apache.accumulo.core.client.SampleNotPresentException;
-import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.data.ByteSequence;
import org.apache.accumulo.core.data.ColumnUpdate;
import org.apache.accumulo.core.data.Key;
@@ -49,13 +46,12 @@ import
org.apache.accumulo.core.iterators.SortedKeyValueIterator;
import org.apache.accumulo.core.iteratorsImpl.system.InterruptibleIterator;
import
org.apache.accumulo.core.iteratorsImpl.system.IterationInterruptedException;
import org.apache.accumulo.core.util.PreAllocatedArray;
+import org.apache.accumulo.tserver.memory.NativeMapLoader;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.annotations.VisibleForTesting;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-
/**
* This class stores data in a C++ map. Doing this allows us to store more in
memory and avoid
* pauses caused by Java GC.
@@ -70,120 +66,9 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
public class NativeMap implements Iterable<Map.Entry<Key,Value>> {
private static final Logger log = LoggerFactory.getLogger(NativeMap.class);
- private static AtomicBoolean loadedNativeLibraries = new
AtomicBoolean(false);
- // Load native library
static {
- // Check in directories set by JVM system property
- List<File> directories = new ArrayList<>();
- String accumuloNativeLibDirs =
System.getProperty("accumulo.native.lib.path");
- if (accumuloNativeLibDirs != null) {
- for (String libDir : accumuloNativeLibDirs.split(":")) {
- directories.add(new File(libDir));
- }
- }
- // Attempt to load from these directories, using standard names
- loadNativeLib(directories);
-
- // Check LD_LIBRARY_PATH (DYLD_LIBRARY_PATH on Mac)
- if (!isLoaded()) {
- if (accumuloNativeLibDirs != null) {
- log.error("Tried and failed to load Accumulo native library from {}",
- accumuloNativeLibDirs);
- }
- String ldLibraryPath = System.getProperty("java.library.path");
- try {
- System.loadLibrary("accumulo");
- loadedNativeLibraries.set(true);
- log.info("Loaded native map shared library from {}", ldLibraryPath);
- } catch (Exception | UnsatisfiedLinkError e) {
- log.error("Tried and failed to load Accumulo native library from {}",
ldLibraryPath, e);
- }
- }
-
- // Exit if native libraries could not be loaded
- if (!isLoaded()) {
- log.error(
- "FATAL! Accumulo native libraries were requested but could not"
- + " be be loaded. Either set '{}' to false in
accumulo.properties or make"
- + " sure native libraries are created in directories set by the
JVM"
- + " system property 'accumulo.native.lib.path' in
accumulo-env.sh!",
- Property.TSERV_NATIVEMAP_ENABLED);
- System.exit(1);
- }
- }
-
- /**
- * If native libraries are not loaded, the specified search path will be
used to attempt to load
- * them. Directories will be searched by using the system-specific library
naming conventions. A
- * path directly to a file can also be provided. Loading will continue until
the search path is
- * exhausted, or until the native libraries are found and successfully
loaded, whichever occurs
- * first.
- *
- * @param searchPath
- * a list of files and directories to search
- */
- @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "search
paths provided by admin")
- public static void loadNativeLib(List<File> searchPath) {
- if (!isLoaded()) {
- List<String> names = getValidLibraryNames();
- List<File> tryList = new ArrayList<>(searchPath.size() * names.size());
-
- for (File p : searchPath)
- if (p.exists() && p.isDirectory())
- for (String name : names)
- tryList.add(new File(p, name));
- else
- tryList.add(p);
-
- for (File f : tryList)
- if (loadNativeLib(f))
- break;
- }
- }
-
- /**
- * Check if native libraries are loaded.
- *
- * @return true if they are loaded; false otherwise
- */
- public static boolean isLoaded() {
- return loadedNativeLibraries.get();
- }
-
- private static List<String> getValidLibraryNames() {
- ArrayList<String> names = new ArrayList<>(3);
-
- String libname = System.mapLibraryName("accumulo");
- names.add(libname);
-
- int dot = libname.lastIndexOf(".");
- String prefix = dot < 0 ? libname : libname.substring(0, dot);
-
- // additional supported Mac extensions
- if ("Mac OS X".equals(System.getProperty("os.name")))
- for (String ext : new String[] {".dylib", ".jnilib"})
- if (!libname.endsWith(ext))
- names.add(prefix + ext);
-
- return names;
- }
-
- private static boolean loadNativeLib(File libFile) {
- log.debug("Trying to load native map library {}", libFile);
- if (libFile.exists() && libFile.isFile()) {
- try {
- System.load(libFile.getAbsolutePath());
- loadedNativeLibraries.set(true);
- log.info("Loaded native map shared library {}", libFile);
- return true;
- } catch (Exception | UnsatisfiedLinkError e) {
- log.error("Tried and failed to load native map library " + libFile, e);
- }
- } else {
- log.debug("Native map library {} not found or is not a file.", libFile);
- }
- return false;
+ NativeMapLoader.load();
}
private final AtomicLong nmPtr = new AtomicLong(0);
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java
index 80ad8e2..69054df 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java
@@ -72,6 +72,7 @@ import org.apache.accumulo.server.ServiceEnvironmentImpl;
import org.apache.accumulo.server.fs.FileManager;
import org.apache.accumulo.server.fs.FileManager.ScanFileManager;
import org.apache.accumulo.tserver.memory.LargestFirstMemoryManager;
+import org.apache.accumulo.tserver.memory.NativeMapLoader;
import org.apache.accumulo.tserver.memory.TabletMemoryReport;
import org.apache.accumulo.tserver.session.ScanSession;
import org.apache.accumulo.tserver.tablet.Tablet;
@@ -253,8 +254,10 @@ public class TabletServerResourceManager {
final AccumuloConfiguration acuConf = context.getConfiguration();
long maxMemory = acuConf.getAsBytes(Property.TSERV_MAXMEM);
- boolean usingNativeMap =
- acuConf.getBoolean(Property.TSERV_NATIVEMAP_ENABLED) &&
NativeMap.isLoaded();
+ boolean usingNativeMap =
acuConf.getBoolean(Property.TSERV_NATIVEMAP_ENABLED);
+ if (usingNativeMap) {
+ NativeMapLoader.load();
+ }
long totalQueueSize =
acuConf.getAsBytes(Property.TSERV_TOTAL_MUTATION_QUEUE_MAX);
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/memory/NativeMapLoader.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/memory/NativeMapLoader.java
new file mode 100644
index 0000000..20d6fdd
--- /dev/null
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/memory/NativeMapLoader.java
@@ -0,0 +1,141 @@
+/*
+ * 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
+ *
+ * http://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.tserver.memory;
+
+import java.io.File;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.accumulo.core.conf.Property;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
+public class NativeMapLoader {
+
+ private static final Logger log =
LoggerFactory.getLogger(NativeMapLoader.class);
+ private static final Pattern dotSuffix = Pattern.compile("[.][^.]*$");
+ private static final String PROP_NAME = "accumulo.native.lib.path";
+ private static final AtomicBoolean loaded = new AtomicBoolean(false);
+
+ // don't allow instantiation
+ private NativeMapLoader() {}
+
+ public synchronized static void load() {
+ // load at most once; System.exit if loading fails
+ if (loaded.compareAndSet(false, true)) {
+ if (loadFromSearchPath(System.getProperty(PROP_NAME)) ||
loadFromSystemLinker()) {
+ return;
+ }
+ log.error(
+ "FATAL! Accumulo native libraries were requested but could not"
+ + " be be loaded. Either set '{}' to false in
accumulo.properties or make"
+ + " sure native libraries are created in directories set by the
JVM"
+ + " system property '{}' in accumulo-env.sh!",
+ Property.TSERV_NATIVEMAP_ENABLED, PROP_NAME);
+ System.exit(1);
+ }
+ }
+
+ public static void loadForTest(List<File> locations, Runnable onFail) {
+ // if the library can't be loaded at the given path, execute the failure
task
+ var searchPath =
locations.stream().map(File::getAbsolutePath).collect(Collectors.joining(":"));
+ if (!loadFromSearchPath(searchPath)) {
+ onFail.run();
+ }
+ }
+
+ /**
+ * The specified search path will be used to attempt to load them.
Directories will be searched by
+ * using the system-specific library naming conventions. A path directly to
a file can also be
+ * provided. Loading will continue until the search path is exhausted, or
until the native
+ * libraries are found and successfully loaded, whichever occurs first.
+ */
+ private static boolean loadFromSearchPath(String searchPath) {
+ // Attempt to load from these directories, using standard names, or by an
exact file name
+ if (searchPath != null) {
+ if
(Stream.of(searchPath.split(":")).flatMap(NativeMapLoader::mapLibraryNames)
+ .anyMatch(NativeMapLoader::loadNativeLib)) {
+ return true;
+ }
+ log.error("Tried and failed to load Accumulo native library from
property {} set to {}",
+ PROP_NAME, searchPath);
+ }
+ return false;
+ }
+
+ // Check LD_LIBRARY_PATH (DYLD_LIBRARY_PATH on Mac)
+ private static boolean loadFromSystemLinker() {
+ String propName = "java.library.path";
+ String ldLibPath = System.getProperty(propName);
+ try {
+ System.loadLibrary("accumulo");
+ log.info("Loaded native map shared library from property {} set to {}",
propName, ldLibPath);
+ return true;
+ } catch (Exception | UnsatisfiedLinkError e) {
+ log.error("Tried and failed to load Accumulo native library from
property {} set to {}",
+ propName, ldLibPath, e);
+ return false;
+ }
+ }
+
+ @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "search
paths provided by admin")
+ private static Stream<File> mapLibraryNames(String name) {
+ File base = new File(name);
+ if (!base.isDirectory()) {
+ return Stream.of(base);
+ }
+ String libname = System.mapLibraryName("accumulo");
+ Stream<String> libs;
+ if ("Mac OS X".equals(System.getProperty("os.name"))) {
+ // additional supported Mac extensions
+ String prefix = dotSuffix.matcher(libname).replaceFirst("");
+ libs = Stream.of(libname, prefix + ".dylib", prefix +
".jnilib").distinct();
+ }
+ libs = Stream.of(libname);
+ return libs.map(f -> appendFileToDir(base, f));
+ }
+
+ // this is its own method because spotbugs sec-bugs doesn't understand how
to suppress lambdas
+ @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "search
paths provided by admin")
+ private static File appendFileToDir(File base, String f) {
+ return new File(base, f);
+ }
+
+ private static boolean loadNativeLib(File libFile) {
+ log.debug("Trying to load native map library {}", libFile);
+ if (libFile.isFile()) {
+ try {
+ System.load(libFile.getAbsolutePath());
+ log.info("Loaded native map shared library {}", libFile);
+ return true;
+ } catch (Exception | UnsatisfiedLinkError e) {
+ log.error("Tried and failed to load native map library {}", libFile,
e);
+ }
+ } else {
+ log.debug("Native map library {} not found or is not a file", libFile);
+ }
+ return false;
+ }
+
+}
diff --git a/test/src/main/java/org/apache/accumulo/test/InMemoryMapIT.java
b/test/src/main/java/org/apache/accumulo/test/InMemoryMapIT.java
index bf34d11..5a3ca3d 100644
--- a/test/src/main/java/org/apache/accumulo/test/InMemoryMapIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/InMemoryMapIT.java
@@ -50,7 +50,7 @@ import org.apache.accumulo.test.categories.SunnyDayTests;
import org.apache.accumulo.test.functional.NativeMapIT;
import org.apache.accumulo.tserver.InMemoryMap;
import org.apache.accumulo.tserver.MemKey;
-import org.apache.accumulo.tserver.NativeMap;
+import org.apache.accumulo.tserver.memory.NativeMapLoader;
import org.easymock.EasyMock;
import org.junit.BeforeClass;
import org.junit.Rule;
@@ -93,13 +93,7 @@ public class InMemoryMapIT {
@BeforeClass
public static void ensureNativeLibrary() {
File nativeMapLocation = NativeMapIT.nativeMapLocation();
- System.setProperty("accumulo.native.lib.path",
nativeMapLocation.getAbsolutePath());
- if (!NativeMap.isLoaded()) {
- fail("Missing the native library from " +
nativeMapLocation.getAbsolutePath()
- + "\nYou need to build the libaccumulo binary first. "
- + "\nTry running 'mvn clean verify -Dit.test=InMemoryMapIT
-Dtest=foo"
- + " -DfailIfNoTests=false -Dspotbugs.skip -Dcheckstyle.skip'");
- }
+ NativeMapLoader.loadForTest(List.of(nativeMapLocation), () -> fail("Can't
load native maps"));
}
public static ServerContext getServerContext() {
diff --git
a/test/src/main/java/org/apache/accumulo/test/functional/ConfigurableMacBase.java
b/test/src/main/java/org/apache/accumulo/test/functional/ConfigurableMacBase.java
index 0ec931c..6a55161 100644
---
a/test/src/main/java/org/apache/accumulo/test/functional/ConfigurableMacBase.java
+++
b/test/src/main/java/org/apache/accumulo/test/functional/ConfigurableMacBase.java
@@ -26,6 +26,7 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.net.InetAddress;
+import java.util.List;
import java.util.Map;
import java.util.Properties;
@@ -40,6 +41,7 @@ import
org.apache.accumulo.miniclusterImpl.ZooKeeperBindException;
import org.apache.accumulo.server.ServerContext;
import org.apache.accumulo.test.categories.MiniClusterOnlyTests;
import org.apache.accumulo.test.util.CertUtils;
+import org.apache.accumulo.tserver.memory.NativeMapLoader;
import org.apache.commons.io.FileUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
@@ -150,13 +152,18 @@ public class ConfigurableMacBase extends AccumuloITBase {
// createTestDir will give us a empty directory, we don't need to clean it
up ourselves
File baseDir = createTestDir(this.getClass().getName() + "_" +
this.testName.getMethodName());
MiniAccumuloConfigImpl cfg = new MiniAccumuloConfigImpl(baseDir,
ROOT_PASSWORD);
- String nativePathInDevTree =
NativeMapIT.nativeMapLocation().getAbsolutePath();
- String nativePathInMapReduce = new
File(System.getProperty("user.dir")).toString();
- cfg.setNativeLibPaths(nativePathInDevTree, nativePathInMapReduce);
+ File nativePathInDevTree = NativeMapIT.nativeMapLocation();
+ File nativePathInMapReduce = new File(System.getProperty("user.dir"));
+ cfg.setNativeLibPaths(nativePathInDevTree.getAbsolutePath(),
nativePathInMapReduce.toString());
Configuration coreSite = new Configuration(false);
cfg.setProperty(Property.TSERV_NATIVEMAP_ENABLED, Boolean.TRUE.toString());
configure(cfg, coreSite);
configureForEnvironment(cfg, getSslDir(baseDir));
+ if
(Boolean.parseBoolean(cfg.getSiteConfig().get(Property.TSERV_NATIVEMAP_ENABLED.getKey())))
{
+ NativeMapLoader.loadForTest(List.of(nativePathInDevTree,
nativePathInMapReduce), () -> {
+ throw new IllegalStateException("Native maps were configured, but not
available");
+ });
+ }
cluster = new MiniAccumuloClusterImpl(cfg);
if (coreSite.size() > 0) {
File csFile = new File(cluster.getConfig().getConfDir(),
"core-site.xml");
diff --git
a/test/src/main/java/org/apache/accumulo/test/functional/NativeMapIT.java
b/test/src/main/java/org/apache/accumulo/test/functional/NativeMapIT.java
index 0f26b93..31f47f1 100644
--- a/test/src/main/java/org/apache/accumulo/test/functional/NativeMapIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/functional/NativeMapIT.java
@@ -33,6 +33,7 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.Iterator;
+import java.util.List;
import java.util.Map.Entry;
import java.util.NoSuchElementException;
import java.util.TreeMap;
@@ -44,6 +45,7 @@ import
org.apache.accumulo.core.iterators.SortedKeyValueIterator;
import org.apache.accumulo.core.util.Pair;
import org.apache.accumulo.test.categories.SunnyDayTests;
import org.apache.accumulo.tserver.NativeMap;
+import org.apache.accumulo.tserver.memory.NativeMapLoader;
import org.apache.hadoop.io.Text;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -77,8 +79,7 @@ public class NativeMapIT {
@BeforeClass
public static void setUp() {
- System.setProperty("accumulo.native.lib.path",
nativeMapLocation().getAbsolutePath());
- assertTrue(NativeMap.isLoaded());
+ NativeMapLoader.loadForTest(List.of(nativeMapLocation()), () ->
fail("Can't load native maps"));
}
private void verifyIterator(int start, int end, int valueOffset,
