[GitHub] [accumulo] ctubbsii commented on issue #2820: Additional improvements to the du command,
ctubbsii commented on issue #2820: URL: https://github.com/apache/accumulo/issues/2820#issuecomment-1234906215 See my comments on the 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2900: Add a new "dumeta" command to compute table disk usage
ctubbsii commented on code in PR #2900: URL: https://github.com/apache/accumulo/pull/2900#discussion_r961154438 ## core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java: ## @@ -1071,6 +1071,24 @@ Map listConstraints(String tableName) List getDiskUsage(Set tables) throws AccumuloException, AccumuloSecurityException, TableNotFoundException; + /** + * Gets the number of bytes being used in the files for a set of tables. This operation will use + * the client to scan the metadata table to compute the size metrics for the tables. For the most + * accurate information a flush or compaction can be run first on the set of tables. Review Comment: And importantly, not all the files have these estimated file sizes. They aren't required. -- 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [accumulo] ctubbsii opened a new pull request, #2912: Updates to ScanAttempts
ctubbsii opened a new pull request, #2912: URL: https://github.com/apache/accumulo/pull/2912 As a follow-up to #2880: * Remove unused and undocumented endTime in ScanAttempt API * Rename ScanAttemptImpl method and variables from "mutationCounter" to "attemptNumber" with corresponding comments to make it clear how this is used for the snapshot * Set attemptNumber in ScanAttemptImpl constructor as an immutable value, rather than setting it immediately after construction * Remove unnecessary synchronization and use an AtomicLong for the currentAttemptNumber * IDE changes also removed some unneeded keywords in the interface and converted an anonymous inner class to a lambda * Create the snapshot using Java streams instead of using "forEach()" -- 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [accumulo] ctubbsii merged pull request #2909: Mark FileSystemMonitor as deprecated and slated for 3.0 removal
ctubbsii merged PR #2909: URL: https://github.com/apache/accumulo/pull/2909 -- 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [accumulo] jmark99 commented on a diff in pull request #2910: Add ability to retrieve TimeType for a table
jmark99 commented on code in PR #2910:
URL: https://github.com/apache/accumulo/pull/2910#discussion_r961077160
##
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java:
##
@@ -2011,6 +2015,34 @@ public ImportDestinationArguments importDirectory(String
directory) {
return new BulkImport(directory, context);
}
+ @Override
+ public TimeType getTimeType(final String tableName) throws
TableNotFoundException {
+if (tableName.equals(RootTable.NAME)) {
+ throw new IllegalArgumentException("accumulo.root table has no
TimeType");
+}
+String systemTableToCheck =
+MetadataTable.NAME.equals(tableName) ? RootTable.NAME :
MetadataTable.NAME;
+final Scanner scanner = context.createScanner(systemTableToCheck,
Authorizations.EMPTY);
+String tableId = tableIdMap().get(tableName);
+if (tableId == null) {
+ throw new TableNotFoundException(null, tableName, "specified table does
not exist");
+}
+final Text start = new Text(tableId);
+final Text end = new Text(start);
+start.append(new byte[] {'<'}, 0, 1);
+end.append(new byte[] {'<'}, 0, 1);
+scanner.setRange(new Range(start, end));
+
MetadataSchema.TabletsSection.ServerColumnFamily.TIME_COLUMN.fetch(scanner);
+Entry next = scanner.iterator().next();
+Value val = next.getValue();
+if (val.toString().startsWith("L")) {
+ return TimeType.LOGICAL;
+} else if (val.toString().startsWith("M")) {
+ return TimeType.MILLIS;
+}
+throw new RuntimeException("Failed to retrieve TimeType");
Review Comment:
@keith-turner thanks for the suggestion. I forgot about the use of ample. I
made a few slight tweaks. 1) forTable requires a TableID vs a String, and 2) I
updated to return a TimeType rather than a TableMetadata object.
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] jmark99 commented on a diff in pull request #2910: Add ability to retrieve TimeType for a table
jmark99 commented on code in PR #2910:
URL: https://github.com/apache/accumulo/pull/2910#discussion_r961075476
##
test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java:
##
@@ -292,4 +295,59 @@ public void
testCompactEmptyTablesWithBadIterator_FailsAndCancel() throws TableE
}
}
+ @Test
+ public void getTimeTypeTest() throws TableNotFoundException,
AccumuloException,
+ TableExistsException, AccumuloSecurityException {
+String[] tableNames = getUniqueNames(4);
+
+// Create table with default MILLIS TimeType
+accumuloClient.tableOperations().create(tableNames[0]);
+TimeType timeType =
accumuloClient.tableOperations().getTimeType(tableNames[0]);
+assertEquals(TimeType.MILLIS, timeType);
+
+// Create table with LOGICAL TimeType.
+NewTableConfiguration ntc = new NewTableConfiguration();
+ntc.setTimeType(TimeType.LOGICAL);
+accumuloClient.tableOperations().create(tableNames[1], ntc);
+timeType = accumuloClient.tableOperations().getTimeType(tableNames[1]);
+assertEquals(TimeType.LOGICAL, timeType);
+
+// Create some split points
+SortedSet splits = new TreeSet<>();
+splits.add(new Text("F"));
+splits.add(new Text("M"));
+splits.add(new Text("S"));
+
+// Create table with MILLIS TimeType. Use splits to create multiple tablets
+ntc = new NewTableConfiguration();
+ntc.withSplits(splits);
+accumuloClient.tableOperations().create(tableNames[2], ntc);
+timeType = accumuloClient.tableOperations().getTimeType(tableNames[2]);
+assertEquals(TimeType.MILLIS, timeType);
+
+// Create table with LOGICAL TimeType. Use splits to create multiple
tablets
+ntc = new NewTableConfiguration();
+ntc.setTimeType(TimeType.LOGICAL).withSplits(splits);
+accumuloClient.tableOperations().create(tableNames[3], ntc);
+timeType = accumuloClient.tableOperations().getTimeType(tableNames[3]);
+assertEquals(TimeType.LOGICAL, timeType);
+
+// check system tables
+timeType =
accumuloClient.tableOperations().getTimeType("accumulo.metadata");
+assertEquals(TimeType.LOGICAL, timeType);
+
+timeType =
accumuloClient.tableOperations().getTimeType("accumulo.replication");
+assertEquals(TimeType.LOGICAL, timeType);
+
+// test non-existent table
+assertThrows(TableNotFoundException.class,
+() -> accumuloClient.tableOperations().getTimeType("notatable"),
+"specified table that doesn't exist");
+
+// cannot get TimeType for root table
+assertThrows(RuntimeException.class,
Review Comment:
With the changes suggested by Keith below this will no longer be necessary.
All tables should return a valid TimeType and a runtime exception will be
thrown if this is not the case.
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] jmark99 commented on a diff in pull request #2910: Add ability to retrieve TimeType for a table
jmark99 commented on code in PR #2910:
URL: https://github.com/apache/accumulo/pull/2910#discussion_r961074258
##
test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java:
##
@@ -292,4 +295,59 @@ public void
testCompactEmptyTablesWithBadIterator_FailsAndCancel() throws TableE
}
}
+ @Test
+ public void getTimeTypeTest() throws TableNotFoundException,
AccumuloException,
+ TableExistsException, AccumuloSecurityException {
+String[] tableNames = getUniqueNames(4);
+
+// Create table with default MILLIS TimeType
+accumuloClient.tableOperations().create(tableNames[0]);
+TimeType timeType =
accumuloClient.tableOperations().getTimeType(tableNames[0]);
+assertEquals(TimeType.MILLIS, timeType);
+
+// Create table with LOGICAL TimeType.
+NewTableConfiguration ntc = new NewTableConfiguration();
+ntc.setTimeType(TimeType.LOGICAL);
+accumuloClient.tableOperations().create(tableNames[1], ntc);
+timeType = accumuloClient.tableOperations().getTimeType(tableNames[1]);
+assertEquals(TimeType.LOGICAL, timeType);
+
+// Create some split points
+SortedSet splits = new TreeSet<>();
+splits.add(new Text("F"));
+splits.add(new Text("M"));
+splits.add(new Text("S"));
+
+// Create table with MILLIS TimeType. Use splits to create multiple tablets
+ntc = new NewTableConfiguration();
+ntc.withSplits(splits);
+accumuloClient.tableOperations().create(tableNames[2], ntc);
Review Comment:
@DomGarguilo I added a comment concerning the default TimeType at table
creation and also created a separate case for explicitly setting the TimeType
to MILLIS.
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] jmark99 commented on a diff in pull request #2910: Add ability to retrieve TimeType for a table
jmark99 commented on code in PR #2910:
URL: https://github.com/apache/accumulo/pull/2910#discussion_r961073453
##
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java:
##
@@ -2011,6 +2015,34 @@ public ImportDestinationArguments importDirectory(String
directory) {
return new BulkImport(directory, context);
}
+ @Override
+ public TimeType getTimeType(final String tableName) throws
TableNotFoundException {
+if (tableName.equals(RootTable.NAME)) {
+ throw new IllegalArgumentException("accumulo.root table has no
TimeType");
+}
+String systemTableToCheck =
+MetadataTable.NAME.equals(tableName) ? RootTable.NAME :
MetadataTable.NAME;
+final Scanner scanner = context.createScanner(systemTableToCheck,
Authorizations.EMPTY);
Review Comment:
With the changes to be made with Keith's suggestion below this change will
no longer be necessary.
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #2197: Per table crypto + other crypto improvements
dlmarion commented on code in PR #2197:
URL: https://github.com/apache/accumulo/pull/2197#discussion_r960956356
##
test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java:
##
@@ -464,7 +464,7 @@ private static int readFiles(VolumeManager fs,
AccumuloConfiguration aconf,
for (TabletFile file : files) {
FileSystem ns = fs.getFileSystemByPath(file.getPath());
FileSKVIterator reader = FileOperations.getInstance().newReaderBuilder()
- .forFile(file.getPathStr(), ns, ns.getConf(),
CryptoServiceFactory.newDefaultInstance())
+ .forFile(file.getPathStr(), ns, ns.getConf(),
NoCryptoServiceFactory.NONE)
Review Comment:
I'm confused how this would work if the File is encrypted.
##
test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java:
##
@@ -497,7 +497,7 @@ private static int readFilesUsingIterStack(VolumeManager
fs, ServerContext conte
for (TabletFile file : files) {
FileSystem ns = fs.getFileSystemByPath(file.getPath());
readers.add(FileOperations.getInstance().newReaderBuilder()
- .forFile(file.getPathStr(), ns, ns.getConf(),
CryptoServiceFactory.newDefaultInstance())
+ .forFile(file.getPathStr(), ns, ns.getConf(),
NoCryptoServiceFactory.NONE)
Review Comment:
I'm confused how this would work if the File is encrypted.
##
server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java:
##
@@ -384,11 +384,11 @@ MetadataTime computeRootTabletTime(ServerContext context,
Collection goo
Path path = new Path(good);
FileSystem ns = context.getVolumeManager().getFileSystemByPath(path);
+var tableConf = context.getTableConfiguration(RootTable.ID);
long maxTime = -1;
try (FileSKVIterator reader =
FileOperations.getInstance().newReaderBuilder()
-.forFile(path.toString(), ns, ns.getConf(),
context.getCryptoService())
-
.withTableConfiguration(context.getTableConfiguration(RootTable.ID)).seekToBeginning()
-.build()) {
+.forFile(path.toString(), ns, ns.getConf(),
tableConf.getCryptoService())
Review Comment:
Should NoCryptoService be used in the upgrade from version 9 to 10?
##
server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java:
##
@@ -522,15 +506,15 @@ public static Map
tryToGetFirstAndLastRows(ServerContext co
}
public static WritableComparable findLastKey(ServerContext context,
- Collection mapFiles) throws IOException {
+ TableConfiguration tableConf, Collection mapFiles) throws
IOException {
Key lastKey = null;
for (TabletFile file : mapFiles) {
FileSystem ns =
context.getVolumeManager().getFileSystemByPath(file.getPath());
FileSKVIterator reader = FileOperations.getInstance().newReaderBuilder()
- .forFile(file.getPathStr(), ns, ns.getConf(),
context.getCryptoService())
-
.withTableConfiguration(context.getConfiguration()).seekToBeginning().build();
+ .forFile(file.getPathStr(), ns, ns.getConf(),
tableConf.getCryptoService())
+ .withTableConfiguration(tableConf).seekToBeginning().build();
Review Comment:
I wonder if `FileOperations` should change, something like:
```
FileOperations.getInstance().newReaderBuilder().table(TableConfiguration)
.file(String, FileSystem, Configuration);
```
##
core/src/main/java/org/apache/accumulo/core/spi/crypto/PerTableCryptoServiceFactory.java:
##
@@ -0,0 +1,118 @@
+/*
+ * 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.core.spi.crypto;
+
+import static java.util.Objects.requireNonNull;
+import static
org.apache.accumulo.core.conf.Property.GENERAL_ARBITRARY_PROP_PREFIX;
+import static org.apache.accumulo.core.conf.Property.TABLE_CRYPTO_PREFIX;
+
+import java.lang.reflect.InvocationTargetException;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.accumulo.core.data.TableId;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A factory that loads a
[GitHub] [accumulo] ctubbsii merged pull request #2911: Serialize location in BadLocationStateException
ctubbsii merged PR #2911: URL: https://github.com/apache/accumulo/pull/2911 -- 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [accumulo-website] jmark99 merged pull request #334: Replace older Tour with newer JShell Tour
jmark99 merged PR #334: URL: https://github.com/apache/accumulo-website/pull/334 -- 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #2906: convert text to printable string before logging in merge tablets
ctubbsii commented on PR #2906: URL: https://github.com/apache/accumulo/pull/2906#issuecomment-1234611631 Thanks for the PR @skirklin . It looks like this is your first contribution to this project. If you wish to be added as a contributor to https://accumulo.apache.org/people/ , please open a pull request to add yourself (in alphabetical order) at https://github.com/apache/accumulo-website/edit/main/pages/people.md and leave a reference to `apache/accumulo#2906` in your commit log. If you intend to be a regular contributor to Accumulo projects, please consider subscribing to our developer mailing list (https://accumulo.apache.org/contact-us/) and introducing yourself. :smiley_cat: -- 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2897: Make Text field transient in BadLocationStateException
ctubbsii commented on code in PR #2897:
URL: https://github.com/apache/accumulo/pull/2897#discussion_r960955209
##
core/src/main/java/org/apache/accumulo/core/metadata/TabletLocationState.java:
##
@@ -36,7 +36,7 @@ public class TabletLocationState {
public static class BadLocationStateException extends Exception {
private static final long serialVersionUID = 1L;
-private Text metadataTableEntry;
+private transient final Text metadataTableEntry;
Review Comment:
Addressed in PR #2911
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] ctubbsii opened a new pull request, #2911: Serialize location in BadLocationStateException
ctubbsii opened a new pull request, #2911: URL: https://github.com/apache/accumulo/pull/2911 To ensure that the location is correctly serialized in this exception, do not mark it transient, but convert it to a byte array, since Text is not Serializable. This addresses the warning in #2776 without assuming the exception will never be serialized, which was made in #2897. Instead, this change avoids errors if it ever were to be serialized and the location were silently changed to a null value because the field was transient. -- 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [accumulo-website] jmark99 commented on a diff in pull request #334: Replace older Tour with newer JShell Tour
jmark99 commented on code in PR #334:
URL: https://github.com/apache/accumulo-website/pull/334#discussion_r960952152
##
tour/batch-scanner-code.md:
##
@@ -4,44 +4,52 @@ title: Batch Scanner Code
Below is a solution to the exercise.
-```java
- static void exercise(AccumuloClient client) throws Exception {
-// Create a table called "GothamPD".
-client.tableOperations().create("GothamPD");
-
-// Generate 10,000 rows of henchman data
-try (BatchWriter writer = client.createBatchWriter("GothamPD")) {
- for (int i = 0; i < 10_000; i++) {
-Mutation m = new Mutation(String.format("id%04d", i));
-m.put("villain", "alias", "henchman" + i);
-m.put("villain", "yearsOfService", "" + (new Random().nextInt(50)));
-m.put("villain", "wearsCape?", "false");
-writer.addMutation(m);
- }
-}
-// 1. Create a BatchScanner with 5 query threads
-try (BatchScanner batchScanner = client.createBatchScanner("GothamPD",
Authorizations.EMPTY, 5)) {
-
- // 2. Create a collection of 2 sample ranges and set it to the
batchScanner
- Listranges = new ArrayList();
- ranges.add(new Range("id1000", "id1999"));
- ranges.add(new Range("id9000", "id"));
- batchScanner.setRanges(ranges);
-
- // 3. Fetch just the columns we want
- batchScanner.fetchColumn(new Text("villain"), new
Text("yearsOfService"));
-
- // 4. Calculate average years of service
- Long totalYears = 0L;
- Long entriesRead = 0L;
- for (Map.Entry entry : batchScanner) {
-totalYears += Long.valueOf(entry.getValue().toString());
-entriesRead++;
- }
- System.out.println("The average years of service of " + entriesRead + "
villains is " + totalYears / entriesRead);
-}
- }
+Create a table called "GothamBatch".
+
+```commandline
+jshell> client.tableOperations().create("GothamBatch");
+```
+
+Generate 10,000 rows of villain data
+
+```commandline
+jshell> try (BatchWriter writer = client.createBatchWriter("GothamBatch")) {
+ ...> for (int i = 0; i < 10_000; i++) {
+ ...> Mutation m = new Mutation(String.format("id%04d", i));
+ ...> m.put("villain", "alias", "henchman" + i);
+ ...> m.put("villain", "yearsOfService", "" + (new
Random().nextInt(50)));
+ ...> m.put("villain", "wearsCape?", "false");
+ ...>writer.addMutation(m);
+ ...> }
+ ...> }
```
+
+Create a BatchScanner with 5 query threads
+```commandline
+jshell> try (BatchScanner batchScanner =
client.createBatchScanner("GothamBatch", Authorizations.EMPTY, 5)) {
+ ...>
+ ...> // Create a collection of 2 sample ranges and set it to the
batchScanner
+ ...> List ranges = new ArrayList();
+ ...>
+ ...> // Create a collection of 2 sample ranges and set it to the
batchScanner
+ ...> ranges.add(new Range("id1000", "id1999"));
+ ...> ranges.add(new Range("id9000", "id"));
+ ...> batchScanner.setRanges(ranges);
+ ...>
+ ...> // Fetch just the columns we want
+ ...> batchScanner.fetchColumn(new Text("villain"), new
Text("yearsOfService"));
+ ...>
+ ...> // Calculate average years of service
+ ...> Long totalYears = 0L;
+ ...> Long entriesRead = 0L;
+ ...> for (Map.Entry entry : batchScanner) {
+ ...> totalYears += Long.valueOf(entry.getValue().toString());
+ ...> entriesRead++;
+ ...> }
+ ...> System.out.println("The average years of service of " + entriesRead
+ " villains is " + totalYears / entriesRead);
Review Comment:
@DomGarguilo I updated the batchScanner to make use of streams as suggested.
Thanks for the suggestion.
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2897: Make Text field transient in BadLocationStateException
ctubbsii commented on code in PR #2897:
URL: https://github.com/apache/accumulo/pull/2897#discussion_r960931792
##
core/src/main/java/org/apache/accumulo/core/metadata/TabletLocationState.java:
##
@@ -36,7 +36,7 @@ public class TabletLocationState {
public static class BadLocationStateException extends Exception {
private static final long serialVersionUID = 1L;
-private Text metadataTableEntry;
+private transient final Text metadataTableEntry;
Review Comment:
I don't know if it is, or will ever be, serialized. But the exceptions are
supposed to always be serializable, because they very well could be at any
time. Others may rely on that for this exception, and won't know to honor the
assumption you made. Having an explicit detection of this case won't make
serialization work... but it will prevent it from introducing bugs that are
hard to find.
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on issue #2908: MiniAccumuloCluster binds admin.serverPort to 8080
ctubbsii commented on issue #2908:
URL: https://github.com/apache/accumulo/issues/2908#issuecomment-1234575819
> Except, duh-uh, of course setting the system property doesn't work -
because MAC spawns ZK in a separate JVM process. Sigh ... I'll have to resort
to one of the uglier work-arounds.
Using non-public API is not recommended because internal code can change
often. However, you might be able to do something like:
```java
var config = new MiniAccumuloConfigImpl(new File("/"), "");
var map = new HashMap<>(config.getSystemProperties());
map.put("zookeeper.admin.enableServer", "false");
config.setSystemProperties(map);
var mac = new MiniAccumuloClusterImpl(config);
mac.start();
```
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] DomGarguilo closed issue #2905: Merging tablets via mergomatic prints unformated Text to terminal
DomGarguilo closed issue #2905: Merging tablets via mergomatic prints unformated Text to terminal URL: https://github.com/apache/accumulo/issues/2905 -- 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [accumulo] DomGarguilo merged pull request #2906: convert text to printable string before logging in merge tablets
DomGarguilo merged PR #2906: URL: https://github.com/apache/accumulo/pull/2906 -- 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [accumulo] keith-turner commented on a diff in pull request #2910: Add ability to retrieve TimeType for a table
keith-turner commented on code in PR #2910:
URL: https://github.com/apache/accumulo/pull/2910#discussion_r960898970
##
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java:
##
@@ -2011,6 +2015,34 @@ public ImportDestinationArguments importDirectory(String
directory) {
return new BulkImport(directory, context);
}
+ @Override
+ public TimeType getTimeType(final String tableName) throws
TableNotFoundException {
+if (tableName.equals(RootTable.NAME)) {
+ throw new IllegalArgumentException("accumulo.root table has no
TimeType");
+}
+String systemTableToCheck =
+MetadataTable.NAME.equals(tableName) ? RootTable.NAME :
MetadataTable.NAME;
+final Scanner scanner = context.createScanner(systemTableToCheck,
Authorizations.EMPTY);
+String tableId = tableIdMap().get(tableName);
+if (tableId == null) {
+ throw new TableNotFoundException(null, tableName, "specified table does
not exist");
+}
+final Text start = new Text(tableId);
+final Text end = new Text(start);
+start.append(new byte[] {'<'}, 0, 1);
+end.append(new byte[] {'<'}, 0, 1);
+scanner.setRange(new Range(start, end));
+
MetadataSchema.TabletsSection.ServerColumnFamily.TIME_COLUMN.fetch(scanner);
+Entry next = scanner.iterator().next();
+Value val = next.getValue();
+if (val.toString().startsWith("L")) {
+ return TimeType.LOGICAL;
+} else if (val.toString().startsWith("M")) {
+ return TimeType.MILLIS;
+}
+throw new RuntimeException("Failed to retrieve TimeType");
Review Comment:
May be able to use ample for this. Also may not need a special case for
root tablet w/ ample, seems like it will return the time type logical. But not
sure, this is the [code that initializes the root tablet
metadata](https://github.com/apache/accumulo/blob/114704bb58a37073a0401c3b6f479761089bd1fb/server/base/src/main/java/org/apache/accumulo/server/init/ZooKeeperInitializer.java#L189-L206)
and makes me think it would return logical.
```suggestion
String tableId = tableIdMap().get(tableName);
if (tableId == null) {
throw new TableNotFoundException(null, tableName, "specified table
does not exist");
}
Optional tabletMetadata =
context.getAmple().readTablets().forTable(tableId)
.fetch(ColumnType.TIME).checkConsistency().build().stream().findFirst();
return tabletMetadata.orElseThrow(() -> new RuntimeException("Failed to
retrieve TimeType"));
```
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] keith-turner commented on a diff in pull request #2910: Add ability to retrieve TimeType for a table
keith-turner commented on code in PR #2910:
URL: https://github.com/apache/accumulo/pull/2910#discussion_r960898970
##
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java:
##
@@ -2011,6 +2015,34 @@ public ImportDestinationArguments importDirectory(String
directory) {
return new BulkImport(directory, context);
}
+ @Override
+ public TimeType getTimeType(final String tableName) throws
TableNotFoundException {
+if (tableName.equals(RootTable.NAME)) {
+ throw new IllegalArgumentException("accumulo.root table has no
TimeType");
+}
+String systemTableToCheck =
+MetadataTable.NAME.equals(tableName) ? RootTable.NAME :
MetadataTable.NAME;
+final Scanner scanner = context.createScanner(systemTableToCheck,
Authorizations.EMPTY);
+String tableId = tableIdMap().get(tableName);
+if (tableId == null) {
+ throw new TableNotFoundException(null, tableName, "specified table does
not exist");
+}
+final Text start = new Text(tableId);
+final Text end = new Text(start);
+start.append(new byte[] {'<'}, 0, 1);
+end.append(new byte[] {'<'}, 0, 1);
+scanner.setRange(new Range(start, end));
+
MetadataSchema.TabletsSection.ServerColumnFamily.TIME_COLUMN.fetch(scanner);
+Entry next = scanner.iterator().next();
+Value val = next.getValue();
+if (val.toString().startsWith("L")) {
+ return TimeType.LOGICAL;
+} else if (val.toString().startsWith("M")) {
+ return TimeType.MILLIS;
+}
+throw new RuntimeException("Failed to retrieve TimeType");
Review Comment:
May be able to use ample for this. Also may not need a special case for
root tablet w/ ample, seems like it will return the time type logical. But not
sure, this is the [code that initializes the root tablet
metadata](https://github.com/apache/accumulo/blob/114704bb58a37073a0401c3b6f479761089bd1fb/server/base/src/main/java/org/apache/accumulo/server/init/ZooKeeperInitializer.java#L189-L206)
and makes me think it would return logical.
```suggestion
String tableId = tableIdMap().get(tableName);
if (tableId == null) {
throw new TableNotFoundException(null, tableName, "specified table
does not exist");
}
Optional tabletMetadata =
context.getAmple().readTablets().forTable(tableId)
.fetch(ColumnType.TIME).checkConsistency().build().stream().findFirst();
return tabletMetadata.orElseThrow(() -> new
RuntimeException(tabletMetadata.orElseThrow(() -> new RuntimeException(;
```
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] keith-turner commented on a diff in pull request #2910: Add ability to retrieve TimeType for a table
keith-turner commented on code in PR #2910:
URL: https://github.com/apache/accumulo/pull/2910#discussion_r960898970
##
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java:
##
@@ -2011,6 +2015,34 @@ public ImportDestinationArguments importDirectory(String
directory) {
return new BulkImport(directory, context);
}
+ @Override
+ public TimeType getTimeType(final String tableName) throws
TableNotFoundException {
+if (tableName.equals(RootTable.NAME)) {
+ throw new IllegalArgumentException("accumulo.root table has no
TimeType");
+}
+String systemTableToCheck =
+MetadataTable.NAME.equals(tableName) ? RootTable.NAME :
MetadataTable.NAME;
+final Scanner scanner = context.createScanner(systemTableToCheck,
Authorizations.EMPTY);
+String tableId = tableIdMap().get(tableName);
+if (tableId == null) {
+ throw new TableNotFoundException(null, tableName, "specified table does
not exist");
+}
+final Text start = new Text(tableId);
+final Text end = new Text(start);
+start.append(new byte[] {'<'}, 0, 1);
+end.append(new byte[] {'<'}, 0, 1);
+scanner.setRange(new Range(start, end));
+
MetadataSchema.TabletsSection.ServerColumnFamily.TIME_COLUMN.fetch(scanner);
+Entry next = scanner.iterator().next();
+Value val = next.getValue();
+if (val.toString().startsWith("L")) {
+ return TimeType.LOGICAL;
+} else if (val.toString().startsWith("M")) {
+ return TimeType.MILLIS;
+}
+throw new RuntimeException("Failed to retrieve TimeType");
Review Comment:
May be able to use ample for this. Also may not need a special case for
root tablet w/ ample, seems like it will return the time type logical. But not
sure, this is the [code that initializes the root tablet
metadata](https://github.com/apache/accumulo/blob/114704bb58a37073a0401c3b6f479761089bd1fb/server/base/src/main/java/org/apache/accumulo/server/init/ZooKeeperInitializer.java#L189-L206)
and makes me think it would return logical.
```suggestion
String tableId = tableIdMap().get(tableName);
if (tableId == null) {
throw new TableNotFoundException(null, tableName, "specified table
does not exist");
}
Optional tabletMetadata =
context.getAmple().readTablets().forTable(tableId)
.fetch(ColumnType.TIME).checkConsistency().build().stream().findFirst();
if(tabletMetadata.isPresent()) {
return tabletMetadata.get().getTime().getType();
} else {
throw new RuntimeException("Failed to retrieve TimeType"))
}
```
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on a diff in pull request #2910: Add ability to retrieve TimeType for a table
DomGarguilo commented on code in PR #2910:
URL: https://github.com/apache/accumulo/pull/2910#discussion_r960850160
##
test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java:
##
@@ -292,4 +295,59 @@ public void
testCompactEmptyTablesWithBadIterator_FailsAndCancel() throws TableE
}
}
+ @Test
+ public void getTimeTypeTest() throws TableNotFoundException,
AccumuloException,
+ TableExistsException, AccumuloSecurityException {
+String[] tableNames = getUniqueNames(4);
+
+// Create table with default MILLIS TimeType
+accumuloClient.tableOperations().create(tableNames[0]);
+TimeType timeType =
accumuloClient.tableOperations().getTimeType(tableNames[0]);
+assertEquals(TimeType.MILLIS, timeType);
+
+// Create table with LOGICAL TimeType.
+NewTableConfiguration ntc = new NewTableConfiguration();
+ntc.setTimeType(TimeType.LOGICAL);
+accumuloClient.tableOperations().create(tableNames[1], ntc);
+timeType = accumuloClient.tableOperations().getTimeType(tableNames[1]);
+assertEquals(TimeType.LOGICAL, timeType);
+
+// Create some split points
+SortedSet splits = new TreeSet<>();
+splits.add(new Text("F"));
+splits.add(new Text("M"));
+splits.add(new Text("S"));
+
+// Create table with MILLIS TimeType. Use splits to create multiple tablets
+ntc = new NewTableConfiguration();
+ntc.withSplits(splits);
+accumuloClient.tableOperations().create(tableNames[2], ntc);
Review Comment:
Maybe a note could be added to the comment about `TimeType.MILLIS` being the
default for ntc and/or another test case could be added which explicitly calls
`ntc.setTimeType(TimeType.MILLIS);`
##
test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java:
##
@@ -292,4 +295,59 @@ public void
testCompactEmptyTablesWithBadIterator_FailsAndCancel() throws TableE
}
}
+ @Test
+ public void getTimeTypeTest() throws TableNotFoundException,
AccumuloException,
+ TableExistsException, AccumuloSecurityException {
+String[] tableNames = getUniqueNames(4);
+
+// Create table with default MILLIS TimeType
+accumuloClient.tableOperations().create(tableNames[0]);
+TimeType timeType =
accumuloClient.tableOperations().getTimeType(tableNames[0]);
+assertEquals(TimeType.MILLIS, timeType);
+
+// Create table with LOGICAL TimeType.
+NewTableConfiguration ntc = new NewTableConfiguration();
+ntc.setTimeType(TimeType.LOGICAL);
+accumuloClient.tableOperations().create(tableNames[1], ntc);
+timeType = accumuloClient.tableOperations().getTimeType(tableNames[1]);
+assertEquals(TimeType.LOGICAL, timeType);
+
+// Create some split points
+SortedSet splits = new TreeSet<>();
+splits.add(new Text("F"));
+splits.add(new Text("M"));
+splits.add(new Text("S"));
+
+// Create table with MILLIS TimeType. Use splits to create multiple tablets
+ntc = new NewTableConfiguration();
+ntc.withSplits(splits);
+accumuloClient.tableOperations().create(tableNames[2], ntc);
+timeType = accumuloClient.tableOperations().getTimeType(tableNames[2]);
+assertEquals(TimeType.MILLIS, timeType);
+
+// Create table with LOGICAL TimeType. Use splits to create multiple
tablets
+ntc = new NewTableConfiguration();
+ntc.setTimeType(TimeType.LOGICAL).withSplits(splits);
+accumuloClient.tableOperations().create(tableNames[3], ntc);
+timeType = accumuloClient.tableOperations().getTimeType(tableNames[3]);
+assertEquals(TimeType.LOGICAL, timeType);
+
+// check system tables
+timeType =
accumuloClient.tableOperations().getTimeType("accumulo.metadata");
+assertEquals(TimeType.LOGICAL, timeType);
+
+timeType =
accumuloClient.tableOperations().getTimeType("accumulo.replication");
+assertEquals(TimeType.LOGICAL, timeType);
+
+// test non-existent table
+assertThrows(TableNotFoundException.class,
+() -> accumuloClient.tableOperations().getTimeType("notatable"),
+"specified table that doesn't exist");
+
+// cannot get TimeType for root table
+assertThrows(RuntimeException.class,
Review Comment:
```suggestion
assertThrows(IllegalArgumentException.class,
```
This could be narrowed down I think
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] milleruntime merged pull request #2904: Fix Fate operation name
milleruntime merged PR #2904: URL: https://github.com/apache/accumulo/pull/2904 -- 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #2910: Add ability to retrieve TimeType for a table
dlmarion commented on code in PR #2910:
URL: https://github.com/apache/accumulo/pull/2910#discussion_r960780534
##
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java:
##
@@ -2011,6 +2015,34 @@ public ImportDestinationArguments importDirectory(String
directory) {
return new BulkImport(directory, context);
}
+ @Override
+ public TimeType getTimeType(final String tableName) throws
TableNotFoundException {
+if (tableName.equals(RootTable.NAME)) {
+ throw new IllegalArgumentException("accumulo.root table has no
TimeType");
+}
+String systemTableToCheck =
+MetadataTable.NAME.equals(tableName) ? RootTable.NAME :
MetadataTable.NAME;
+final Scanner scanner = context.createScanner(systemTableToCheck,
Authorizations.EMPTY);
Review Comment:
suggest moving this line to right before scanner is used and put in
try-with-resources block.
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [accumulo] jmark99 commented on pull request #2910: Add ability to retrieve TimeType for a table
jmark99 commented on PR #2910: URL: https://github.com/apache/accumulo/pull/2910#issuecomment-1234413246 Ticket replaces Jira issue https://issues.apache.org/jira/browse/ACCUMULO-3447 -- 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Resolved] (ACCUMULO-3447) No way to programmatically get table's TimeType
[ https://issues.apache.org/jira/browse/ACCUMULO-3447?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Mark Owens resolved ACCUMULO-3447. -- Resolution: Done Closing ticket as it is replaced by Accumulo GitHub issue #2910 (Add ability to retrieve TimeType for a table). > No way to programmatically get table's TimeType > --- > > Key: ACCUMULO-3447 > URL: https://issues.apache.org/jira/browse/ACCUMULO-3447 > Project: Accumulo > Issue Type: Improvement > Components: client >Reporter: John Vines >Priority: Major > > Looking through the APIs to try to determine a table's time type and it seems > like there's no mechanism for it -- This message was sent by Atlassian Jira (v8.20.10#820010)
[GitHub] [accumulo] jmark99 opened a new pull request, #2910: Add ability to retrieve TimeType for a table
jmark99 opened a new pull request, #2910: URL: https://github.com/apache/accumulo/pull/2910 Provide a way to programmatically retrieve a table's TimeType value. Added client method 'gettimetype' and corresponding shell command that retrieves the TimeType for a supplied table. -- 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [accumulo] EdColeman opened a new pull request, #2909: Mark FileSystemMonitor as deprecated and slated for 3.0 removal
EdColeman opened a new pull request, #2909: URL: https://github.com/apache/accumulo/pull/2909 For 3.0, PR #1328 will remove the FileSystemMonitor that monitors the mounted file systems and can kill the server if a disk goes read-only. With WALs using hdfs, the primary driver for this action is not necessary. Also, the current FileSystemMonitor code uses a limited set of file systems and supports etx3, ext4 and xfs and would need to be updated for newer file systems such as btrfs. -- 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-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
