> On Oct. 30, 2014, 4:06 p.m., kturner wrote: > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CloneIntoTable.java, > > line 93 > > <https://reviews.apache.org/r/27198/diff/2/?file=741888#file741888line93> > > > > This is user unfriendly. We could possibly add the needed splits to > > the dest table.
I would be more content keeping these (clone into & split) in two seperate operations. Theis makes the implications to the calling user more explicit and, in the longer term, I see this operation being updated to not require splits to line up, which will be a change in behavior that wouldn't be backward compatible if we make that adjustment. > On Oct. 30, 2014, 4:06 p.m., kturner wrote: > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CloneIntoTable.java, > > line 134 > > <https://reviews.apache.org/r/27198/diff/2/?file=741888#file741888line134> > > > > AFAICT nothing is done w/ the logical time of the src table? Should > > take the max logical time for a src tablet and dest tablet and set that as > > the dest tablets logical time. Also logical times should be of same type. Having issues with this one as there seems to be no mechanisms in fate (or the client API in general) to get a table's time type. Do you know of a way to do this in fate? > On Oct. 30, 2014, 4:06 p.m., kturner wrote: > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CloneIntoTable.java, > > line 84 > > <https://reviews.apache.org/r/27198/diff/2/?file=741888#file741888line84> > > > > Are the tables offline? If not, then the splits could change during > > this operation. They are not, but this isn't an issue. The locks prevent a merge and the bulk import code we're utilizing handles tablets splitting mid-operation. On Oct. 30, 2014, 4:06 p.m., John Vines wrote: > > I have only reviewed the FATE ops so far. How will this work w/ > > replication? > > > > I am thinking another possible approach may be to make a clone operation > > that accepts multiple input tables and creates a new table. The reason I > > am thinking about this is that it avoids having to deal w/ issues related > > to the dest table changing while the clone is happening. Something like > > > > clone([tableA, tableB], tableC) > > > > However, this is stil still tricky. The existing clone handles cloning of > > online table that may be splitting. It makes multiple passes over the src > > table metadata entries updating the dest until it stabilizes. In order to > > avoid this for multiple tables could move from a cloneInto that supports > > multiple online inputs to adding a merge that supports multiple offline > > tables as input. > > > > ``` > > clone(tableA, tmpA) > > offline(tmpA) > > clone(tableB, tmpB) > > offline(tmpB) > > mergeTables([tmpA, tmpB], tmpC) > > ``` > > > > After this tmpA and tmpB would be deleted and tmpC would have the files and > > splits of both. tmpC would also have the correct logical time. However one > > thing I am not sure about is about per table props. When clone(tableA, > > tableB) is done, it will create tableB w/ tableA's per table props. > > John Vines wrote: > Please refer to the JIRA about this feature, which explains why I need > this feature to not be going into a new table > > kturner wrote: > ok. I still assert that this feature will be tricky to get correct :) > But I think it can be done. > > One concurrency issue that I am not sure is handled in this patch is the > following. > > 1. read files for src table (includes fileA) > 1. fileA is dereferenced by src table > 1. fileA is garbage collected > 1. a referernce to fileA is written to dest table > > This is one case the current clone table code handles. Added bulk load style import in progress markers to block deletion tracking extent->file mapping via file now - John ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27198/#review59201 ----------------------------------------------------------- On Oct. 29, 2014, 8:52 p.m., John Vines wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27198/ > ----------------------------------------------------------- > > (Updated Oct. 29, 2014, 8:52 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-3236 > https://issues.apache.org/jira/browse/ACCUMULO-3236 > > > Repository: accumulo > > > Description > ------- > > Includes all code to support feature, including thrift changes > Includes minor code cleanup to TableLocator and items in the Bulk path to > remove signature items that are unused (arguments & exceptions) > Includes renaming of some bulk import functions to clarify their purpose > (because they're now multi-purpose) > > Patch is based on 1.6, but we can choose to make it target only 1.7 if we > choose (this conversation should be taken up on jira, not in RB) > > > Diffs > ----- > > > core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java > 97f538d > > core/src/main/java/org/apache/accumulo/core/client/impl/RootTabletLocator.java > 97d476b > > core/src/main/java/org/apache/accumulo/core/client/impl/TableOperationsImpl.java > 2792bcc > core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocator.java > e396d82 > > core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocatorImpl.java > c550f15 > > core/src/main/java/org/apache/accumulo/core/client/impl/TimeoutTabletLocator.java > bcbe561 > > core/src/main/java/org/apache/accumulo/core/client/impl/thrift/TableOperation.java > 7716823 > > core/src/main/java/org/apache/accumulo/core/client/mock/MockTableOperationsImpl.java > de19137 > > core/src/main/java/org/apache/accumulo/core/client/mock/impl/MockTabletLocator.java > 35f160f > > core/src/main/java/org/apache/accumulo/core/master/thrift/FateOperation.java > f65f552 > > core/src/main/java/org/apache/accumulo/core/tabletserver/thrift/TabletClientService.java > 2ba7674 > core/src/main/thrift/client.thrift 38a8076 > core/src/main/thrift/master.thrift 38e9227 > core/src/main/thrift/tabletserver.thrift 25e0b10 > > core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java > 1d91574 > > core/src/test/java/org/apache/accumulo/core/client/impl/TableOperationsHelperTest.java > 02838ed > > server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java > 27ab078 > > server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java > ebea064 > > server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java > d0e6aea > > server/base/src/test/java/org/apache/accumulo/server/client/BulkImporterTest.java > 3680341 > > server/master/src/main/java/org/apache/accumulo/master/FateServiceHandler.java > 5818da3 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CloneIntoTable.java > PRE-CREATION > server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java > 0778f5b > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java > 03fe069 > > test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java > 0591b19 > test/src/test/java/org/apache/accumulo/test/functional/CloneIntoIT.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/27198/diff/ > > > Testing > ------- > > Includes CloneIntoIT, which exercises all permutations of the flags. Existing > BulkIT still functions as intended for validation of no feature loss in > refactoring exiting code for multi-purposing. > > > Thanks, > > John Vines > >
