You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/01/14 20:45:44 UTC
[GitHub] [accumulo] ivakegg opened a new pull request #1475: fixes #1474:
Option to leave cloned tables offline
ivakegg opened a new pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475
* Added an optional keepOffline option to TableOperations.clone
* Updated the shell clone command with a -k option
* Updated the CloneTestIT to test the keepOffline option
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r367728873
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
Keeping the change minimal in 1.x is a nice goal, but adding an overloaded method now, that we won't need in 2.1 because the builder is added to 2.1 seems to only add unnecessary bloat. It would probably make sense to add the builder for both 1.10 and 2.1, or only add this entire feature to 2.1, so we never need to add the overloaded version of the method anywhere.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on issue #1475: fixes #1474:
Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#issuecomment-579468120
Merged into master 0ff1542c6261bf65c529c39e85d9a5448eb6bc65
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371865390
##########
File path: test/src/main/java/org/apache/accumulo/test/functional/CloneTestIT.java
##########
@@ -121,6 +125,12 @@ public void testProps() throws Exception {
}
+ private void checkTableState(String table, Connector c, TableState expected) throws TException {
Review comment:
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1475: fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371269465
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfigurationImpl.java
##########
@@ -0,0 +1,123 @@
+/*
+ * 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.core.client.admin;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+
+import com.google.common.base.Preconditions;
+
+/**
+ * A {@link CloneConfiguration} implementation which also implements the builder thereof
+ *
+ * @since 1.10 and 2.1
+ */
+public class CloneConfigurationImpl implements CloneConfiguration, CloneConfiguration.Builder {
+
+ // The purpose of this is to allow building an immutable CloneConfiguration object without
+ // creating
+ // separate Builder and CloneConfiguration objects. This is done to reduce object creation and
+ // copying. This could easily be changed to two objects without changing the interfaces.
+ private boolean built = false;
+
+ // determines if memory is flushed in the source table before cloning.
+ private boolean flush = true;
+
+ // the sources table properties are copied, this allows overriding of those properties
+ private Map<String,String> propertiesToSet = null;
+
+ // do not copy these properties from the source table, just revert to system defaults
+ private Set<String> propertiesToExclude = null;
+
+ // do not bring the table online after cloning
+ private boolean keepOffline = false;
+
+ private CloneConfigurationImpl() {}
+
+ public boolean isFlush() {
+ Preconditions.checkState(built);
+ return flush;
+ }
+
+ public Map<String,String> getPropertiesToSet() {
+ Preconditions.checkState(built);
+ return (propertiesToSet == null ? Collections.<String,String>emptyMap()
+ : Collections.unmodifiableMap(propertiesToSet));
+ }
+
+ public Set<String> getPropertiesToExclude() {
+ Preconditions.checkState(built);
+ return (propertiesToExclude == null ? Collections.<String>emptySet()
+ : Collections.unmodifiableSet(propertiesToExclude));
+ }
+
+ public boolean isKeepOffline() {
+ Preconditions.checkState(built);
+ return keepOffline;
+ }
+
+ @Override
+ public Builder setFlush(boolean flush) {
+ Preconditions.checkState(!built);
+ this.flush = flush;
+ return this;
+ }
+
+ @Override
+ public Builder setPropertiesToSet(Map<String,String> propertiesToSet) {
+ Preconditions.checkState(!built);
+ this.propertiesToSet = propertiesToSet;
+ return this;
+ }
+
+ @Override
+ public Builder setPropertiesToExclude(Set<String> propertiesToExclude) {
+ Preconditions.checkState(!built);
+ this.propertiesToExclude = propertiesToExclude;
+ return this;
+ }
+
+ @Override
+ public Builder setKeepOffline(boolean keepOffline) {
+ Preconditions.checkState(!built);
+ this.keepOffline = keepOffline;
+ return this;
+ }
+
+ @Override
+ public CloneConfiguration build() {
+ Preconditions.checkState(!built);
+ built = true;
+ return this;
+ }
+
+ @Override
+ public String toString() {
+ return "{flush=" + flush + ", propertiesToSet=" + propertiesToSet + ", propertiesToExclude="
+ + propertiesToExclude + ", keepOffline=" + keepOffline + ", built=" + built + "}";
+ }
+
+ /**
+ * @return a {@link CloneConfiguration} builder
+ */
+ public static CloneConfiguration.Builder builder() {
Review comment:
This method should be in the ```CloneConfiguration``` interface so users don't need to touch the Impl class. With Java 1.8 you can make this a ```default``` method too.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r370675268
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
ok, adding into the 1.10 line. Change will be coming soon.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371819658
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
Ah, I see it now. So should I change to the same pattern for the builder()? I guess I would at least be able to make the CloneConfigurationImpl package private if I did but I am not sure it is worth the extra class.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r367730767
##########
File path: core/src/main/java/org/apache/accumulo/core/client/mock/MockTableOperations.java
##########
@@ -474,6 +474,14 @@ public void clone(String srcTableName, String newTableName, boolean flush,
throw new NotImplementedException();
}
+ @Override
+ public void clone(String srcTableName, String newTableName, boolean flush,
+ Map<String,String> propertiesToSet, Set<String> propertiesToExclude, boolean keepOffline)
+ throws AccumuloException, AccumuloSecurityException, TableNotFoundException,
+ TableExistsException {
+ throw new NotImplementedException();
Review comment:
INFO-only: The standard built-in Java exception here is `UnsupportedOperationException`, not the commons-lang subclass. This is fine for consistency with the rest of the class, but it's a bad idea to use the commons-lang version in general (especially since it has been added and removed there, so it depends on what version of the library you're using). We replaced all these occurrences that in the 2.x branch when we switched to commons-lang3.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371849503
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
OK, then I will leave it as is. Thanks @keith-turner.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r370675748
##########
File path: core/src/main/java/org/apache/accumulo/core/client/mock/MockTableOperations.java
##########
@@ -474,6 +474,14 @@ public void clone(String srcTableName, String newTableName, boolean flush,
throw new NotImplementedException();
}
+ @Override
+ public void clone(String srcTableName, String newTableName, boolean flush,
+ Map<String,String> propertiesToSet, Set<String> propertiesToExclude, boolean keepOffline)
+ throws AccumuloException, AccumuloSecurityException, TableNotFoundException,
+ TableExistsException {
+ throw new NotImplementedException();
Review comment:
ok, I will have to remember this when I merge into 2.1.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] keith-turner commented on a change in pull request
#1475: fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r366572641
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
Review comment:
need a since 2.1 javadoc tag
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1475: fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r366582891
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
Review comment:
I think he is targeting the 1.9 branch, which will be 1.10. So I guess it should be a since 1.10 javadoc tag, which may be the only one haha.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371388958
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfigurationImpl.java
##########
@@ -0,0 +1,123 @@
+/*
+ * 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.core.client.admin;
Review comment:
ah, ok
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] keith-turner commented on a change in pull request
#1475: fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r366585753
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
Review comment:
Interesting, API in 1.10, not in 2.0, and in 2.1.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on issue #1475: fixes #1474:
Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#issuecomment-579292744
> @milleruntime Thanks. I will rebase and collapse the commits and them merge.
No problem... there are some other commits in 1.9 that need to be methodically merged up anyway
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371389816
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfigurationImpl.java
##########
@@ -0,0 +1,123 @@
+/*
+ * 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.core.client.admin;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+
+import com.google.common.base.Preconditions;
+
+/**
+ * A {@link CloneConfiguration} implementation which also implements the builder thereof
+ *
+ * @since 1.10 and 2.1
+ */
+public class CloneConfigurationImpl implements CloneConfiguration, CloneConfiguration.Builder {
+
+ // The purpose of this is to allow building an immutable CloneConfiguration object without
+ // creating
+ // separate Builder and CloneConfiguration objects. This is done to reduce object creation and
+ // copying. This could easily be changed to two objects without changing the interfaces.
+ private boolean built = false;
+
+ // determines if memory is flushed in the source table before cloning.
+ private boolean flush = true;
+
+ // the sources table properties are copied, this allows overriding of those properties
+ private Map<String,String> propertiesToSet = null;
+
+ // do not copy these properties from the source table, just revert to system defaults
+ private Set<String> propertiesToExclude = null;
+
+ // do not bring the table online after cloning
+ private boolean keepOffline = false;
+
+ private CloneConfigurationImpl() {}
+
+ public boolean isFlush() {
+ Preconditions.checkState(built);
+ return flush;
+ }
+
+ public Map<String,String> getPropertiesToSet() {
+ Preconditions.checkState(built);
+ return (propertiesToSet == null ? Collections.<String,String>emptyMap()
+ : Collections.unmodifiableMap(propertiesToSet));
+ }
+
+ public Set<String> getPropertiesToExclude() {
+ Preconditions.checkState(built);
+ return (propertiesToExclude == null ? Collections.<String>emptySet()
+ : Collections.unmodifiableSet(propertiesToExclude));
+ }
+
+ public boolean isKeepOffline() {
+ Preconditions.checkState(built);
+ return keepOffline;
+ }
+
+ @Override
+ public Builder setFlush(boolean flush) {
+ Preconditions.checkState(!built);
+ this.flush = flush;
+ return this;
+ }
+
+ @Override
+ public Builder setPropertiesToSet(Map<String,String> propertiesToSet) {
+ Preconditions.checkState(!built);
+ this.propertiesToSet = propertiesToSet;
+ return this;
+ }
+
+ @Override
+ public Builder setPropertiesToExclude(Set<String> propertiesToExclude) {
+ Preconditions.checkState(!built);
+ this.propertiesToExclude = propertiesToExclude;
+ return this;
+ }
+
+ @Override
+ public Builder setKeepOffline(boolean keepOffline) {
+ Preconditions.checkState(!built);
+ this.keepOffline = keepOffline;
+ return this;
+ }
+
+ @Override
+ public CloneConfiguration build() {
+ Preconditions.checkState(!built);
+ built = true;
+ return this;
+ }
+
+ @Override
+ public String toString() {
+ return "{flush=" + flush + ", propertiesToSet=" + propertiesToSet + ", propertiesToExclude="
+ + propertiesToExclude + ", keepOffline=" + keepOffline + ", built=" + built + "}";
+ }
+
+ /**
+ * @return a {@link CloneConfiguration} builder
+ */
+ public static CloneConfiguration.Builder builder() {
Review comment:
If the pom was not forcing 1.7 source compatibility, then I would. In the top level pom of the 1.9 branch I see:
<maven.compiler.release>7</maven.compiler.release>
<maven.compiler.source>1.7</maven.compiler.source>
<maven.compiler.target>1.7</maven.compiler.target>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r366602735
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
I like it, but perhaps I do this only in the 2.1 line?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime merged pull request #1475: fixes #1474:
Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
milleruntime merged pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r370789248
##########
File path: core/src/main/java/org/apache/accumulo/core/client/mock/MockTableOperations.java
##########
@@ -474,6 +474,14 @@ public void clone(String srcTableName, String newTableName, boolean flush,
throw new NotImplementedException();
}
+ @Override
+ public void clone(String srcTableName, String newTableName, boolean flush,
+ Map<String,String> propertiesToSet, Set<String> propertiesToExclude, boolean keepOffline)
+ throws AccumuloException, AccumuloSecurityException, TableNotFoundException,
+ TableExistsException {
+ throw new NotImplementedException();
Review comment:
Actually there is no MockTableOperations in 2.1, so this is a moot point.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] Manno15 commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
Manno15 commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r367042842
##########
File path: shell/src/main/java/org/apache/accumulo/shell/commands/CloneTableCommand.java
##########
@@ -99,6 +105,8 @@ public Options getOptions() {
noFlushOption =
new Option("nf", "noFlush", false, "do not flush table data in memory before cloning.");
o.addOption(noFlushOption);
+ keepOfflineOption =
+ new Option("o", "offline", false, "do not bring the table online after cloning.");
Review comment:
keepOfflineOption wasn't added to 'o' so it won't show up on the shell command line as an option.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] drewfarris commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
drewfarris commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371862013
##########
File path: test/src/main/java/org/apache/accumulo/test/functional/CloneTestIT.java
##########
@@ -121,6 +125,12 @@ public void testProps() throws Exception {
}
+ private void checkTableState(String table, Connector c, TableState expected) throws TException {
Review comment:
Purely pedantic, so I'm sure this is ignorable - consider naming this `assertTableState` because it actually performs the assertion and will fail if the assertion fails.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371404739
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfigurationImpl.java
##########
@@ -0,0 +1,123 @@
+/*
+ * 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.core.client.admin;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+
+import com.google.common.base.Preconditions;
+
+/**
+ * A {@link CloneConfiguration} implementation which also implements the builder thereof
+ *
+ * @since 1.10 and 2.1
+ */
+public class CloneConfigurationImpl implements CloneConfiguration, CloneConfiguration.Builder {
+
+ // The purpose of this is to allow building an immutable CloneConfiguration object without
+ // creating
+ // separate Builder and CloneConfiguration objects. This is done to reduce object creation and
+ // copying. This could easily be changed to two objects without changing the interfaces.
+ private boolean built = false;
+
+ // determines if memory is flushed in the source table before cloning.
+ private boolean flush = true;
+
+ // the sources table properties are copied, this allows overriding of those properties
+ private Map<String,String> propertiesToSet = null;
+
+ // do not copy these properties from the source table, just revert to system defaults
+ private Set<String> propertiesToExclude = null;
+
+ // do not bring the table online after cloning
+ private boolean keepOffline = false;
+
+ private CloneConfigurationImpl() {}
+
+ public boolean isFlush() {
+ Preconditions.checkState(built);
+ return flush;
+ }
+
+ public Map<String,String> getPropertiesToSet() {
+ Preconditions.checkState(built);
+ return (propertiesToSet == null ? Collections.<String,String>emptyMap()
+ : Collections.unmodifiableMap(propertiesToSet));
+ }
+
+ public Set<String> getPropertiesToExclude() {
+ Preconditions.checkState(built);
+ return (propertiesToExclude == null ? Collections.<String>emptySet()
+ : Collections.unmodifiableSet(propertiesToExclude));
+ }
+
+ public boolean isKeepOffline() {
+ Preconditions.checkState(built);
+ return keepOffline;
+ }
+
+ @Override
+ public Builder setFlush(boolean flush) {
+ Preconditions.checkState(!built);
+ this.flush = flush;
+ return this;
+ }
+
+ @Override
+ public Builder setPropertiesToSet(Map<String,String> propertiesToSet) {
+ Preconditions.checkState(!built);
+ this.propertiesToSet = propertiesToSet;
+ return this;
+ }
+
+ @Override
+ public Builder setPropertiesToExclude(Set<String> propertiesToExclude) {
+ Preconditions.checkState(!built);
+ this.propertiesToExclude = propertiesToExclude;
+ return this;
+ }
+
+ @Override
+ public Builder setKeepOffline(boolean keepOffline) {
+ Preconditions.checkState(!built);
+ this.keepOffline = keepOffline;
+ return this;
+ }
+
+ @Override
+ public CloneConfiguration build() {
+ Preconditions.checkState(!built);
+ built = true;
+ return this;
+ }
+
+ @Override
+ public String toString() {
+ return "{flush=" + flush + ", propertiesToSet=" + propertiesToSet + ", propertiesToExclude="
+ + propertiesToExclude + ", keepOffline=" + keepOffline + ", built=" + built + "}";
+ }
+
+ /**
+ * @return a {@link CloneConfiguration} builder
+ */
+ public static CloneConfiguration.Builder builder() {
Review comment:
I see it now.... ok moving static method into interface
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371417910
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
I have implemented a CloneConfiguration and CloneConfigurationImpl. However I left the impl public to enable creating a new instance in the CloneConfiguration.builder() implementation. Looking at your ScanDirectives.builder(), it appears to always return the same instance of a builder and hence the configuration object. Hence that is really not usable across threads. Was that intentional?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] keith-turner commented on a change in pull request
#1475: fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r366578323
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
Would be nice to create a simple class for clone configuration so in the future we do not need to add more overloaded methods. Something like the following.
```java
/**
* @since 2.1.0
*/
void clone(String srcTableName, String newTableName, CloneConfiguration config);
```
Could possibly have the following API for CloneConfiguration.
```java
/**
* @since 2.1.0
*/
public class CloneConfiguration {
Map<String,String> getPropertiesToSet();
Set<String> getPropertiesToExclude();
boolean flush();
InitialTableState getInitialState();
/**
* @since 2.1.0
*/
public static interface Builder {
/**
* props to set on the clone. If not set defaults to empty map.
*/
Builder setProperties(Map<String,String> props);
/**
* Props to exclude form the source table. If not set defaults to empty set.
*
*/
Builder excludeProperties(Set<String> props);
/**
* The initial table state for the clone. If not set defaults to ONLINE.
*/
Builder setInitialTableState(InitialTableState init);
/**
* Determines if source tables in memory data is flushed before cloning. Unflushed data will not be in clone. If not set defaults to true.
*/
Builder setFlush(boolean flush);
// builds an immutable CloneConfiguration
CloneConfiguration build();
}
public static Builder builder() {
// TODO return a new builder obj that is not visible in the public API.. see ScanDirectivesImpl
// for an example of simple way to implement
return null;
}
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371417910
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
@keith-turner : I have implemented a CloneConfiguration and CloneConfigurationImpl. However I left the impl public to enable creating a new instance in the CloneConfiguration.builder() implementation. Looking at your ScanDirectives.builder(), it appears to always return the same instance of a builder and hence the configuration object. Hence that is really not usable across threads. Was that intentional?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1475: fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371499528
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
@ivakegg I think I figured out what is going on with the ScanDirectives.builder()... the setters in DefaultScanDirectives will return a new object of the parent class.
```return new ScanDirectivesImpl().setExecutorName(name);```
So the builder will always return ```DefaultScanDirectives``` first with the ```builder()``` method and then only if something needs to be set return a new ```ScanDirectivesImpl``` 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1475: fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371440333
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
FYI ScanDirectives were added in https://github.com/apache/accumulo/pull/1440
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1475: fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r369122899
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
I agree. There shouldn't be anything in this change that can't go into 1.10 (now that it is java 1.8). It should then be fairly easy to merge forward and no need for #1476
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1475: fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371393473
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfigurationImpl.java
##########
@@ -0,0 +1,123 @@
+/*
+ * 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.core.client.admin;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+
+import com.google.common.base.Preconditions;
+
+/**
+ * A {@link CloneConfiguration} implementation which also implements the builder thereof
+ *
+ * @since 1.10 and 2.1
+ */
+public class CloneConfigurationImpl implements CloneConfiguration, CloneConfiguration.Builder {
+
+ // The purpose of this is to allow building an immutable CloneConfiguration object without
+ // creating
+ // separate Builder and CloneConfiguration objects. This is done to reduce object creation and
+ // copying. This could easily be changed to two objects without changing the interfaces.
+ private boolean built = false;
+
+ // determines if memory is flushed in the source table before cloning.
+ private boolean flush = true;
+
+ // the sources table properties are copied, this allows overriding of those properties
+ private Map<String,String> propertiesToSet = null;
+
+ // do not copy these properties from the source table, just revert to system defaults
+ private Set<String> propertiesToExclude = null;
+
+ // do not bring the table online after cloning
+ private boolean keepOffline = false;
+
+ private CloneConfigurationImpl() {}
+
+ public boolean isFlush() {
+ Preconditions.checkState(built);
+ return flush;
+ }
+
+ public Map<String,String> getPropertiesToSet() {
+ Preconditions.checkState(built);
+ return (propertiesToSet == null ? Collections.<String,String>emptyMap()
+ : Collections.unmodifiableMap(propertiesToSet));
+ }
+
+ public Set<String> getPropertiesToExclude() {
+ Preconditions.checkState(built);
+ return (propertiesToExclude == null ? Collections.<String>emptySet()
+ : Collections.unmodifiableSet(propertiesToExclude));
+ }
+
+ public boolean isKeepOffline() {
+ Preconditions.checkState(built);
+ return keepOffline;
+ }
+
+ @Override
+ public Builder setFlush(boolean flush) {
+ Preconditions.checkState(!built);
+ this.flush = flush;
+ return this;
+ }
+
+ @Override
+ public Builder setPropertiesToSet(Map<String,String> propertiesToSet) {
+ Preconditions.checkState(!built);
+ this.propertiesToSet = propertiesToSet;
+ return this;
+ }
+
+ @Override
+ public Builder setPropertiesToExclude(Set<String> propertiesToExclude) {
+ Preconditions.checkState(!built);
+ this.propertiesToExclude = propertiesToExclude;
+ return this;
+ }
+
+ @Override
+ public Builder setKeepOffline(boolean keepOffline) {
+ Preconditions.checkState(!built);
+ this.keepOffline = keepOffline;
+ return this;
+ }
+
+ @Override
+ public CloneConfiguration build() {
+ Preconditions.checkState(!built);
+ built = true;
+ return this;
+ }
+
+ @Override
+ public String toString() {
+ return "{flush=" + flush + ", propertiesToSet=" + propertiesToSet + ", propertiesToExclude="
+ + propertiesToExclude + ", keepOffline=" + keepOffline + ", built=" + built + "}";
+ }
+
+ /**
+ * @return a {@link CloneConfiguration} builder
+ */
+ public static CloneConfiguration.Builder builder() {
Review comment:
Probably need to update your branch... I added Java 1.8 in 9c957ab55f28f10dde1321eb613a9ec3e2fa5322 for the 1.10 release.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r367731058
##########
File path: shell/src/main/java/org/apache/accumulo/shell/commands/CloneTableCommand.java
##########
@@ -99,6 +105,8 @@ public Options getOptions() {
noFlushOption =
new Option("nf", "noFlush", false, "do not flush table data in memory before cloning.");
o.addOption(noFlushOption);
+ keepOfflineOption =
+ new Option("o", "offline", false, "do not bring the table online after cloning.");
Review comment:
This could be tested with ShellServerIT
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r367728104
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
Yeah, keeping the change minimal in 1.x is best.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r367728104
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
Yeah, keeping the change minimal in 1.x is best.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r370676306
##########
File path: shell/src/main/java/org/apache/accumulo/shell/commands/CloneTableCommand.java
##########
@@ -99,6 +105,8 @@ public Options getOptions() {
noFlushOption =
new Option("nf", "noFlush", false, "do not flush table data in memory before cloning.");
o.addOption(noFlushOption);
+ keepOfflineOption =
+ new Option("o", "offline", false, "do not bring the table online after cloning.");
Review comment:
doh
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] keith-turner commented on a change in pull request
#1475: fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371839894
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
+
+ void clone(String srcTableName, String newTableName, boolean flush,
Review comment:
The pattern I chose for ScanDirectives builder is complex because I thought it would be called very frequently and wanted it to be efficient. For CloneConfiguration builder, I don't think efficiency would be a concern so personally I would go with the simplest implementation.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r366602588
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
Review comment:
so what should I do in this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r370673943
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
Review comment:
will do
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1475: fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r366578980
##########
File path: shell/src/main/java/org/apache/accumulo/shell/commands/CloneTableCommand.java
##########
@@ -99,6 +105,8 @@ public Options getOptions() {
noFlushOption =
new Option("nf", "noFlush", false, "do not flush table data in memory before cloning.");
o.addOption(noFlushOption);
+ keepOfflineOption =
+ new Option("k", "keepOffline", false, "do not bring the table online after cloning.");
Review comment:
Probably better to name the option "o", to be consistent with other offline commands. For example, that is what [CreateTableCommand](https://github.com/apache/accumulo/blob/3fd5cad92f9b63ac19e4466f3f2d5237b905262c/shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java#L328) uses in 2.X
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1475: fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r371265851
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfigurationImpl.java
##########
@@ -0,0 +1,123 @@
+/*
+ * 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.core.client.admin;
Review comment:
The Impl class should be in the client.impl package.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] keith-turner commented on a change in pull request
#1475: fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r366606006
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
Review comment:
I am not sure... maybe it could be `@since 1.10 2.1.0`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r367727871
##########
File path: core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
##########
@@ -433,6 +433,33 @@ void clone(String srcTableName, String newTableName, boolean flush,
Map<String,String> propertiesToSet, Set<String> propertiesToExclude) throws AccumuloException,
AccumuloSecurityException, TableNotFoundException, TableExistsException;
+ /**
+ * Clone a table from an existing table. The cloned table will have the same data as the source
+ * table it was created from. After cloning, the two tables can mutate independently. Initially
+ * the cloned table should not use any extra space, however as the source table and cloned table
+ * major compact extra space will be used by the clone.
+ *
+ * Initially the cloned table is only readable and writable by the user who created it.
+ *
+ * @param srcTableName
+ * the table to clone
+ * @param newTableName
+ * the name of the clone
+ * @param flush
+ * determines if memory is flushed in the source table before cloning.
+ * @param propertiesToSet
+ * the sources tables properties are copied, this allows overriding of those properties
+ * @param propertiesToExclude
+ * do not copy these properties from the source table, just revert to system defaults
+ * @param keepOffline
+ * do not bring the table online after cloning
+ */
Review comment:
`@since 1.10.0 and 2.1.0` works. Or, since we don't generally add new APIs in bugfix/patch releases, it might suffice to put `@since 1.10 and 2.1`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ivakegg commented on a change in pull request #1475:
fixes #1474: Option to leave cloned tables offline
Posted by GitBox <gi...@apache.org>.
ivakegg commented on a change in pull request #1475: fixes #1474: Option to leave cloned tables offline
URL: https://github.com/apache/accumulo/pull/1475#discussion_r366602397
##########
File path: shell/src/main/java/org/apache/accumulo/shell/commands/CloneTableCommand.java
##########
@@ -99,6 +105,8 @@ public Options getOptions() {
noFlushOption =
new Option("nf", "noFlush", false, "do not flush table data in memory before cloning.");
o.addOption(noFlushOption);
+ keepOfflineOption =
+ new Option("k", "keepOffline", false, "do not bring the table online after cloning.");
Review comment:
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services