keith-turner commented on code in PR #3409: URL: https://github.com/apache/accumulo/pull/3409#discussion_r1202691175
########## server/base/src/main/java/org/apache/accumulo/server/manager/state/ManagerTabletInfoIterator.java: ########## @@ -0,0 +1,452 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.accumulo.server.manager.state; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Base64; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.SortedMap; +import java.util.stream.Collectors; + +import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.client.ScannerBase; +import org.apache.accumulo.core.client.admin.TabletHostingGoal; +import org.apache.accumulo.core.conf.ConfigurationTypeHelper; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.data.ByteSequence; +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Range; +import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.dataImpl.KeyExtent; +import org.apache.accumulo.core.iterators.IteratorEnvironment; +import org.apache.accumulo.core.iterators.SkippingIterator; +import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.core.iterators.user.WholeRowIterator; +import org.apache.accumulo.core.manager.state.ManagerTabletInfo; +import org.apache.accumulo.core.manager.state.ManagerTabletInfo.ManagementAction; +import org.apache.accumulo.core.manager.thrift.ManagerState; +import org.apache.accumulo.core.metadata.TServerInstance; +import org.apache.accumulo.core.metadata.TabletState; +import org.apache.accumulo.core.metadata.schema.DataFileValue; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ChoppedColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.CurrentLocationColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.FutureLocationColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.HostingColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LastLocationColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LogColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SuspendLocationColumn; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily; +import org.apache.accumulo.core.metadata.schema.TabletMetadata; +import org.apache.accumulo.core.metadata.schema.TabletMetadata.Location; +import org.apache.accumulo.core.metadata.schema.TabletMetadata.LocationType; +import org.apache.accumulo.core.util.AddressUtil; +import org.apache.hadoop.io.DataInputBuffer; +import org.apache.hadoop.io.DataOutputBuffer; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.base.Joiner; +import com.google.common.collect.Sets; + +public class ManagerTabletInfoIterator extends SkippingIterator { + + private static final Logger LOG = LoggerFactory.getLogger(ManagerTabletInfoIterator.class); + + private static final String SERVERS_OPTION = "servers"; + private static final String TABLES_OPTION = "tables"; + private static final String MERGES_OPTION = "merges"; + private static final String DEBUG_OPTION = "debug"; + private static final String MIGRATIONS_OPTION = "migrations"; + private static final String MANAGER_STATE_OPTION = "managerState"; + private static final String SHUTTING_DOWN_OPTION = "shuttingDown"; + + private static void setCurrentServers(final IteratorSetting cfg, + final Set<TServerInstance> goodServers) { + if (goodServers != null) { + List<String> servers = new ArrayList<>(); + for (TServerInstance server : goodServers) { + servers.add(server.getHostPortSession()); + } + cfg.addOption(SERVERS_OPTION, Joiner.on(",").join(servers)); + } + } + + private static void setOnlineTables(final IteratorSetting cfg, final Set<TableId> onlineTables) { + if (onlineTables != null) { + cfg.addOption(TABLES_OPTION, Joiner.on(",").join(onlineTables)); + } + } + + private static void setMerges(final IteratorSetting cfg, final Collection<MergeInfo> merges) { + DataOutputBuffer buffer = new DataOutputBuffer(); + try { + for (MergeInfo info : merges) { + KeyExtent extent = info.getExtent(); + if (extent != null && !info.getState().equals(MergeState.NONE)) { + info.write(buffer); + } + } + } catch (Exception ex) { + throw new RuntimeException(ex); + } + String encoded = + Base64.getEncoder().encodeToString(Arrays.copyOf(buffer.getData(), buffer.getLength())); + cfg.addOption(MERGES_OPTION, encoded); + } + + private static void setMigrations(final IteratorSetting cfg, + final Collection<KeyExtent> migrations) { + DataOutputBuffer buffer = new DataOutputBuffer(); + try { + for (KeyExtent extent : migrations) { + extent.writeTo(buffer); + } + } catch (Exception ex) { + throw new RuntimeException(ex); + } + String encoded = + Base64.getEncoder().encodeToString(Arrays.copyOf(buffer.getData(), buffer.getLength())); + cfg.addOption(MIGRATIONS_OPTION, encoded); + } + + private static void setManagerState(final IteratorSetting cfg, final ManagerState state) { + cfg.addOption(MANAGER_STATE_OPTION, state.toString()); + } + + private static void setShuttingDown(final IteratorSetting cfg, + final Set<TServerInstance> servers) { + if (servers != null) { + cfg.addOption(SHUTTING_DOWN_OPTION, Joiner.on(",").join(servers)); + } + } + + private static Set<KeyExtent> parseMigrations(final String migrations) { + if (migrations == null) { + return Collections.emptySet(); + } + try { + Set<KeyExtent> result = new HashSet<>(); + DataInputBuffer buffer = new DataInputBuffer(); + byte[] data = Base64.getDecoder().decode(migrations); + buffer.reset(data, data.length); + while (buffer.available() > 0) { + result.add(KeyExtent.readFrom(buffer)); + } + return result; + } catch (Exception ex) { + throw new RuntimeException(ex); + } + } + + private static Set<TableId> parseTableIDs(final String tableIDs) { + if (tableIDs == null) { + return null; + } + Set<TableId> result = new HashSet<>(); + for (String tableID : tableIDs.split(",")) { + result.add(TableId.of(tableID)); + } + return result; + } + + private static Set<TServerInstance> parseServers(final String servers) { + if (servers == null) { + return null; + } + // parse "host:port[INSTANCE]" + Set<TServerInstance> result = new HashSet<>(); + if (!servers.isEmpty()) { + for (String part : servers.split(",")) { + String[] parts = part.split("\\[", 2); + String hostport = parts[0]; + String instance = parts[1]; + if (instance != null && instance.endsWith("]")) { + instance = instance.substring(0, instance.length() - 1); + } + result.add(new TServerInstance(AddressUtil.parseAddress(hostport, false), instance)); + } + } + return result; + } + + private static Map<TableId,MergeInfo> parseMerges(final String merges) { + if (merges == null) { + return null; + } + try { + Map<TableId,MergeInfo> result = new HashMap<>(); + DataInputBuffer buffer = new DataInputBuffer(); + byte[] data = Base64.getDecoder().decode(merges); + buffer.reset(data, data.length); + while (buffer.available() > 0) { + MergeInfo mergeInfo = new MergeInfo(); + mergeInfo.readFields(buffer); + result.put(mergeInfo.extent.tableId(), mergeInfo); + } + return result; + } catch (Exception ex) { + throw new RuntimeException(ex); + } + } + + private static boolean shouldReturnDueToSplit(final TabletMetadata tm, + final long splitThreshold) { + return tm.getFilesMap().values().stream().map(DataFileValue::getSize) + .collect(Collectors.summarizingLong(Long::longValue)).getSum() > splitThreshold; + } + + private static boolean shouldReturnDueToLocation(final TabletMetadata tm, + final Set<TableId> onlineTables, final Set<TServerInstance> current, final boolean debug) { + // is the table supposed to be online or offline? + final boolean shouldBeOnline = onlineTables.contains(tm.getTableId()); + + if (debug) { + LOG.debug("{} is {}. Table is {}line. Tablet hosting goal is {}, hostingRequested: {}", + tm.getExtent(), getLocationState(current, tm), (shouldBeOnline ? "on" : "off"), + tm.getHostingGoal(), tm.getHostingRequested()); + } + switch (getLocationState(current, tm)) { + case ASSIGNED: + // we always want data about assigned tablets + return true; + case HOSTED: + if (!shouldBeOnline || tm.getHostingGoal() == TabletHostingGoal.NEVER + || (tm.getHostingGoal() == TabletHostingGoal.ONDEMAND && !tm.getHostingRequested())) { + return true; + } + break; + case ASSIGNED_TO_DEAD_SERVER: + return true; + case SUSPENDED: + case UNASSIGNED: + if (shouldBeOnline && (tm.getHostingGoal() == TabletHostingGoal.ALWAYS + || (tm.getHostingGoal() == TabletHostingGoal.ONDEMAND && tm.getHostingRequested()))) { + return true; + } + break; + default: + throw new AssertionError( + "Inconceivable! The tablet is an unrecognized state: " + getLocationState(current, tm)); + } + return false; + } + + private static TabletState getLocationState(final Set<TServerInstance> liveServers, + final TabletMetadata tm) { + Location loc = tm.getLocation(); + + if (loc == null || loc.getType() == null || loc.getServerInstance() == null) { + return TabletState.UNASSIGNED; + } + + if (loc.getType().equals(LocationType.FUTURE)) { + return liveServers.contains(loc.getServerInstance()) ? TabletState.ASSIGNED + : TabletState.ASSIGNED_TO_DEAD_SERVER; + } else if (loc.getType().equals(LocationType.CURRENT)) { + return liveServers.contains(loc.getServerInstance()) ? TabletState.HOSTED + : TabletState.ASSIGNED_TO_DEAD_SERVER; + } else if (tm.getSuspend() != null) { + return TabletState.SUSPENDED; + } else { + return TabletState.UNASSIGNED; + } + } Review Comment: ```suggestion ``` Could this method be dropped and TabletMetadata.getTabletState() used instead? ########## core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java: ########## @@ -285,21 +285,22 @@ private ScanServerData binRanges(ClientTabletCache clientTabletCache, List<Range break; } else { // tried to only do table state checks when failures.size() == ranges.size(), however this - // did - // not work because nothing ever invalidated entries in the tabletLocator cache... so even - // though - // the table was deleted the tablet locator entries for the deleted table were not - // cleared... so - // need to always do the check when failures occur + // did not work because nothing ever invalidated entries in the tabletLocator cache... so + // even though the table was deleted the tablet locator entries for the deleted table + // were not cleared. So need to always do the check when failures occur if (failures.size() >= lastFailureSize) { context.requireNotDeleted(tableId); context.requireNotOffline(tableId, tableName); } lastFailureSize = failures.size(); + log.debug( Review Comment: Is this meant to be left here? ########## server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java: ########## @@ -219,13 +240,24 @@ public void run() { // towards their goal iter = store.iterator(); while (iter.hasNext()) { - TabletLocationState tls = iter.next(); - if (tls == null) { + final ManagerTabletInfo mti = iter.next(); + if (mti == null) { + LOG.debug("ManagerTabletInfoIterator returned null ManagerTabletInfo"); Review Comment: Is it returning null expected? ########## core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java: ########## @@ -379,6 +384,20 @@ public boolean getHostingRequested() { return onDemandHostingRequested; } + @Override + public String toString() { Review Comment: This is really nice to have. ########## server/base/src/test/java/org/apache/accumulo/server/manager/state/TabletMetadataIteratorTest.java: ########## @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.accumulo.server.manager.state; + +import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.COMPACT_COLUMN; +import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN; +import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.FLUSH_COLUMN; +import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.TIME_COLUMN; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.ArrayList; +import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; + +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Mutation; +import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.dataImpl.KeyExtent; +import org.apache.accumulo.core.fate.FateTxId; +import org.apache.accumulo.core.iterators.user.WholeRowIterator; +import org.apache.accumulo.core.manager.state.ManagerTabletInfo; +import org.apache.accumulo.core.manager.state.ManagerTabletInfo.ManagementAction; +import org.apache.accumulo.core.metadata.StoredTabletFile; +import org.apache.accumulo.core.metadata.schema.DataFileValue; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.BulkFileColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ClonedColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.CurrentLocationColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LastLocationColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ScanFileColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily; +import org.apache.accumulo.core.tabletserver.log.LogEntry; +import org.apache.hadoop.io.Text; +import org.junit.jupiter.api.Test; + +public class TabletMetadataIteratorTest { + + private SortedMap<Key,Value> toRowMap(Mutation mutation) { + SortedMap<Key,Value> rowMap = new TreeMap<>(); + mutation.getUpdates().forEach(cu -> { + Key k = new Key(mutation.getRow(), cu.getColumnFamily(), cu.getColumnQualifier(), + cu.getTimestamp()); + Value v = new Value(cu.getValue()); + rowMap.put(k, v); + }); + return rowMap; + } + + private SortedMap<Key,Value> createMetadataEntryKV() { + KeyExtent extent = new KeyExtent(TableId.of("5"), new Text("df"), new Text("da")); + + Mutation mutation = TabletColumnFamily.createPrevRowMutation(extent); + + COMPACT_COLUMN.put(mutation, new Value("5")); + DIRECTORY_COLUMN.put(mutation, new Value("t-0001757")); + FLUSH_COLUMN.put(mutation, new Value("6")); + TIME_COLUMN.put(mutation, new Value("M123456789")); + + String bf1 = "hdfs://nn1/acc/tables/1/t-0001/bf1"; + String bf2 = "hdfs://nn1/acc/tables/1/t-0001/bf2"; + mutation.at().family(BulkFileColumnFamily.NAME).qualifier(bf1).put(FateTxId.formatTid(56)); + mutation.at().family(BulkFileColumnFamily.NAME).qualifier(bf2).put(FateTxId.formatTid(59)); + + mutation.at().family(ClonedColumnFamily.NAME).qualifier("").put("OK"); + + DataFileValue dfv1 = new DataFileValue(555, 23); + StoredTabletFile tf1 = new StoredTabletFile("hdfs://nn1/acc/tables/1/t-0001/df1.rf"); + StoredTabletFile tf2 = new StoredTabletFile("hdfs://nn1/acc/tables/1/t-0001/df2.rf"); + mutation.at().family(DataFileColumnFamily.NAME).qualifier(tf1.getMetaUpdateDelete()) + .put(dfv1.encode()); + DataFileValue dfv2 = new DataFileValue(234, 13); + mutation.at().family(DataFileColumnFamily.NAME).qualifier(tf2.getMetaUpdateDelete()) + .put(dfv2.encode()); + + mutation.at().family(CurrentLocationColumnFamily.NAME).qualifier("s001").put("server1:8555"); + + mutation.at().family(LastLocationColumnFamily.NAME).qualifier("s000").put("server2:8555"); + + LogEntry le1 = new LogEntry(extent, 55, "lf1"); + mutation.at().family(le1.getColumnFamily()).qualifier(le1.getColumnQualifier()) + .timestamp(le1.timestamp).put(le1.getValue()); + LogEntry le2 = new LogEntry(extent, 57, "lf2"); + mutation.at().family(le2.getColumnFamily()).qualifier(le2.getColumnQualifier()) + .timestamp(le2.timestamp).put(le2.getValue()); + + StoredTabletFile sf1 = new StoredTabletFile("hdfs://nn1/acc/tables/1/t-0001/sf1.rf"); + StoredTabletFile sf2 = new StoredTabletFile("hdfs://nn1/acc/tables/1/t-0001/sf2.rf"); + mutation.at().family(ScanFileColumnFamily.NAME).qualifier(sf1.getMetaUpdateDelete()).put(""); + mutation.at().family(ScanFileColumnFamily.NAME).qualifier(sf2.getMetaUpdateDelete()).put(""); + + return toRowMap(mutation); + + } + + @Test + public void testEncodeDecodeWithReasons() throws Exception { + final Set<ManagementAction> reasons = Review Comment: ```suggestion final Set<ManagementAction> actions = ``` ########## server/base/src/test/java/org/apache/accumulo/server/manager/state/TabletMetadataIteratorTest.java: ########## @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.accumulo.server.manager.state; + +import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.COMPACT_COLUMN; +import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN; +import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.FLUSH_COLUMN; +import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.TIME_COLUMN; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.ArrayList; +import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; + +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Mutation; +import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.dataImpl.KeyExtent; +import org.apache.accumulo.core.fate.FateTxId; +import org.apache.accumulo.core.iterators.user.WholeRowIterator; +import org.apache.accumulo.core.manager.state.ManagerTabletInfo; +import org.apache.accumulo.core.manager.state.ManagerTabletInfo.ManagementAction; +import org.apache.accumulo.core.metadata.StoredTabletFile; +import org.apache.accumulo.core.metadata.schema.DataFileValue; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.BulkFileColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ClonedColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.CurrentLocationColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LastLocationColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ScanFileColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily; +import org.apache.accumulo.core.tabletserver.log.LogEntry; +import org.apache.hadoop.io.Text; +import org.junit.jupiter.api.Test; + +public class TabletMetadataIteratorTest { Review Comment: Test focuses on code in ManagerTabletInfo, maybe it should be in the test name ```suggestion public class ManagerTabletInfoTest { ``` ########## server/base/src/test/java/org/apache/accumulo/server/manager/state/TabletMetadataImposterTest.java: ########## @@ -18,171 +18,171 @@ */ package org.apache.accumulo.server.manager.state; -import static org.easymock.EasyMock.createMock; -import static org.easymock.EasyMock.expect; -import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.createNiceMock; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.util.Collection; +import java.util.ArrayList; +import java.util.List; import java.util.Set; import org.apache.accumulo.core.client.admin.TabletHostingGoal; import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.core.metadata.TServerInstance; -import org.apache.accumulo.core.metadata.TabletLocationState; import org.apache.accumulo.core.metadata.TabletState; +import org.apache.accumulo.core.metadata.schema.TabletMetadata; import org.apache.accumulo.core.metadata.schema.TabletMetadata.Location; -import org.apache.hadoop.io.Text; -import org.junit.jupiter.api.BeforeAll; +import org.apache.accumulo.core.tabletserver.log.LogEntry; +import org.easymock.EasyMock; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -public class CachedTabletStateTest { - private static final Collection<String> innerWalogs = new java.util.HashSet<>(); - private static final Collection<Collection<String>> walogs = new java.util.HashSet<>(); - - @BeforeAll - public static void setUpClass() { - walogs.add(innerWalogs); - innerWalogs.add("somelog"); - } +public class TabletMetadataImposterTest { private KeyExtent keyExtent; private Location future; private Location current; private Location last; - private TabletLocationState tls; + private TabletMetadata tls; + private List<LogEntry> walogs = new ArrayList<>(); @BeforeEach public void setUp() { - keyExtent = createMock(KeyExtent.class); - future = Location.future(createMock(TServerInstance.class)); - current = Location.current(createMock(TServerInstance.class)); - last = Location.last(createMock(TServerInstance.class)); + keyExtent = createNiceMock(KeyExtent.class); + TServerInstance futureInstance = createNiceMock(TServerInstance.class); + future = Location.future(futureInstance); + TServerInstance currentInstance = createNiceMock(TServerInstance.class); + current = Location.current(currentInstance); + TServerInstance lastInstance = createNiceMock(TServerInstance.class); + last = Location.last(lastInstance); + walogs.clear(); + walogs.add(new LogEntry(keyExtent, 1234L, "some file name")); + EasyMock.replay(keyExtent, futureInstance, currentInstance, lastInstance); } @Test public void testConstruction_NoFuture() throws Exception { - tls = new TabletLocationState(keyExtent, null, current, last, null, walogs, true, + tls = new TabletMetadataImposter(keyExtent, null, current, last, null, walogs, true, TabletHostingGoal.ONDEMAND, false); - assertSame(keyExtent, tls.extent); - assertNull(tls.future); - assertSame(current, tls.current); - assertSame(last, tls.last); - assertSame(walogs, tls.walogs); - assertTrue(tls.chopped); + assertSame(keyExtent, tls.getExtent()); + assertTrue(tls.hasCurrent()); + assertSame(current, tls.getLocation()); + assertSame(last, tls.getLast()); + assertSame(walogs, tls.getLogs()); + assertTrue(tls.hasChopped()); } @Test public void testConstruction_NoCurrent() throws Exception { - tls = new TabletLocationState(keyExtent, future, null, last, null, walogs, true, + tls = new TabletMetadataImposter(keyExtent, future, null, last, null, walogs, true, TabletHostingGoal.ONDEMAND, false); - assertSame(keyExtent, tls.extent); - assertSame(future, tls.future); - assertNull(tls.current); - assertSame(last, tls.last); - assertSame(walogs, tls.walogs); - assertTrue(tls.chopped); + assertSame(keyExtent, tls.getExtent()); + assertSame(future, tls.getLocation()); + assertFalse(tls.hasCurrent()); + assertSame(last, tls.getLast()); + assertSame(walogs, tls.getLogs()); + assertTrue(tls.hasChopped()); } @Test public void testConstruction_FutureAndCurrent() { - expect(keyExtent.toMetaRow()).andReturn(new Text("entry")); - replay(keyExtent); - var e = assertThrows(TabletLocationState.BadLocationStateException.class, - () -> new TabletLocationState(keyExtent, future, current, last, null, walogs, true, - TabletHostingGoal.ONDEMAND, false)); - assertEquals(new Text("entry"), e.getEncodedEndRow()); + tls = new TabletMetadataImposter(keyExtent, future, current, last, null, walogs, true, + TabletHostingGoal.ONDEMAND, false); + assertTrue(tls.isFutureAndCurrentLocationSet()); } @Test public void testConstruction_NoFuture_NoWalogs() throws Exception { - tls = new TabletLocationState(keyExtent, null, current, last, null, null, true, + tls = new TabletMetadataImposter(keyExtent, null, current, last, null, null, true, TabletHostingGoal.ONDEMAND, false); - assertNotNull(tls.walogs); - assertEquals(0, tls.walogs.size()); + assertNotNull(tls.getLogs()); + assertEquals(0, tls.getLogs().size()); } @Test public void testGetServer_Current() throws Exception { - tls = new TabletLocationState(keyExtent, null, current, last, null, walogs, true, + tls = new TabletMetadataImposter(keyExtent, null, current, last, null, walogs, true, TabletHostingGoal.ONDEMAND, false); + assertTrue(tls.hasCurrent()); assertSame(current, tls.getLocation()); } @Test public void testGetServer_Future() throws Exception { - tls = new TabletLocationState(keyExtent, future, null, last, null, walogs, true, + tls = new TabletMetadataImposter(keyExtent, future, null, last, null, walogs, true, TabletHostingGoal.ONDEMAND, false); + assertFalse(tls.hasCurrent()); assertSame(future, tls.getLocation()); } @Test public void testGetServer_Last() throws Exception { - tls = new TabletLocationState(keyExtent, null, null, last, null, walogs, true, + tls = new TabletMetadataImposter(keyExtent, null, null, last, null, walogs, true, TabletHostingGoal.ONDEMAND, false); - assertSame(last, tls.getLocation()); + assertFalse(tls.hasCurrent()); + assertNull(tls.getLocation()); + assertSame(last, tls.getLast()); } @Test public void testGetServer_None() throws Exception { - tls = new TabletLocationState(keyExtent, null, null, null, null, walogs, true, + tls = new TabletMetadataImposter(keyExtent, null, null, null, null, walogs, true, TabletHostingGoal.ONDEMAND, false); + assertFalse(tls.hasCurrent()); assertNull(tls.getLocation()); } @Test public void testGetState_Unassigned1() throws Exception { - tls = new TabletLocationState(keyExtent, null, null, null, null, walogs, true, + tls = new TabletMetadataImposter(keyExtent, null, null, null, null, walogs, true, TabletHostingGoal.ONDEMAND, false); - assertEquals(TabletState.UNASSIGNED, tls.getState(null)); + assertEquals(TabletState.UNASSIGNED, tls.getTabletState(null)); Review Comment: This is a preexisting problem. I tried running this test locally and noticed the suspended state is not covered by this test. ########## server/base/src/main/java/org/apache/accumulo/server/manager/state/ManagerTabletInfoIterator.java: ########## @@ -0,0 +1,452 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.accumulo.server.manager.state; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Base64; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.SortedMap; +import java.util.stream.Collectors; + +import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.client.ScannerBase; +import org.apache.accumulo.core.client.admin.TabletHostingGoal; +import org.apache.accumulo.core.conf.ConfigurationTypeHelper; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.data.ByteSequence; +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Range; +import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.dataImpl.KeyExtent; +import org.apache.accumulo.core.iterators.IteratorEnvironment; +import org.apache.accumulo.core.iterators.SkippingIterator; +import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.core.iterators.user.WholeRowIterator; +import org.apache.accumulo.core.manager.state.ManagerTabletInfo; +import org.apache.accumulo.core.manager.state.ManagerTabletInfo.ManagementAction; +import org.apache.accumulo.core.manager.thrift.ManagerState; +import org.apache.accumulo.core.metadata.TServerInstance; +import org.apache.accumulo.core.metadata.TabletState; +import org.apache.accumulo.core.metadata.schema.DataFileValue; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ChoppedColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.CurrentLocationColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.FutureLocationColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.HostingColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LastLocationColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LogColumnFamily; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SuspendLocationColumn; +import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily; +import org.apache.accumulo.core.metadata.schema.TabletMetadata; +import org.apache.accumulo.core.metadata.schema.TabletMetadata.Location; +import org.apache.accumulo.core.metadata.schema.TabletMetadata.LocationType; +import org.apache.accumulo.core.util.AddressUtil; +import org.apache.hadoop.io.DataInputBuffer; +import org.apache.hadoop.io.DataOutputBuffer; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.base.Joiner; +import com.google.common.collect.Sets; + +public class ManagerTabletInfoIterator extends SkippingIterator { + + private static final Logger LOG = LoggerFactory.getLogger(ManagerTabletInfoIterator.class); + + private static final String SERVERS_OPTION = "servers"; + private static final String TABLES_OPTION = "tables"; + private static final String MERGES_OPTION = "merges"; + private static final String DEBUG_OPTION = "debug"; + private static final String MIGRATIONS_OPTION = "migrations"; + private static final String MANAGER_STATE_OPTION = "managerState"; + private static final String SHUTTING_DOWN_OPTION = "shuttingDown"; + + private static void setCurrentServers(final IteratorSetting cfg, + final Set<TServerInstance> goodServers) { + if (goodServers != null) { + List<String> servers = new ArrayList<>(); + for (TServerInstance server : goodServers) { + servers.add(server.getHostPortSession()); + } + cfg.addOption(SERVERS_OPTION, Joiner.on(",").join(servers)); + } + } + + private static void setOnlineTables(final IteratorSetting cfg, final Set<TableId> onlineTables) { + if (onlineTables != null) { + cfg.addOption(TABLES_OPTION, Joiner.on(",").join(onlineTables)); + } + } + + private static void setMerges(final IteratorSetting cfg, final Collection<MergeInfo> merges) { + DataOutputBuffer buffer = new DataOutputBuffer(); + try { + for (MergeInfo info : merges) { + KeyExtent extent = info.getExtent(); + if (extent != null && !info.getState().equals(MergeState.NONE)) { + info.write(buffer); + } + } + } catch (Exception ex) { + throw new RuntimeException(ex); + } + String encoded = + Base64.getEncoder().encodeToString(Arrays.copyOf(buffer.getData(), buffer.getLength())); + cfg.addOption(MERGES_OPTION, encoded); + } + + private static void setMigrations(final IteratorSetting cfg, + final Collection<KeyExtent> migrations) { + DataOutputBuffer buffer = new DataOutputBuffer(); + try { + for (KeyExtent extent : migrations) { + extent.writeTo(buffer); + } + } catch (Exception ex) { + throw new RuntimeException(ex); + } + String encoded = + Base64.getEncoder().encodeToString(Arrays.copyOf(buffer.getData(), buffer.getLength())); + cfg.addOption(MIGRATIONS_OPTION, encoded); + } + + private static void setManagerState(final IteratorSetting cfg, final ManagerState state) { + cfg.addOption(MANAGER_STATE_OPTION, state.toString()); + } + + private static void setShuttingDown(final IteratorSetting cfg, + final Set<TServerInstance> servers) { + if (servers != null) { + cfg.addOption(SHUTTING_DOWN_OPTION, Joiner.on(",").join(servers)); + } + } + + private static Set<KeyExtent> parseMigrations(final String migrations) { + if (migrations == null) { + return Collections.emptySet(); + } + try { + Set<KeyExtent> result = new HashSet<>(); + DataInputBuffer buffer = new DataInputBuffer(); + byte[] data = Base64.getDecoder().decode(migrations); + buffer.reset(data, data.length); + while (buffer.available() > 0) { + result.add(KeyExtent.readFrom(buffer)); + } + return result; + } catch (Exception ex) { + throw new RuntimeException(ex); + } + } + + private static Set<TableId> parseTableIDs(final String tableIDs) { + if (tableIDs == null) { + return null; + } + Set<TableId> result = new HashSet<>(); + for (String tableID : tableIDs.split(",")) { + result.add(TableId.of(tableID)); + } + return result; + } + + private static Set<TServerInstance> parseServers(final String servers) { + if (servers == null) { + return null; + } + // parse "host:port[INSTANCE]" + Set<TServerInstance> result = new HashSet<>(); + if (!servers.isEmpty()) { + for (String part : servers.split(",")) { + String[] parts = part.split("\\[", 2); + String hostport = parts[0]; + String instance = parts[1]; + if (instance != null && instance.endsWith("]")) { + instance = instance.substring(0, instance.length() - 1); + } + result.add(new TServerInstance(AddressUtil.parseAddress(hostport, false), instance)); + } + } + return result; + } + + private static Map<TableId,MergeInfo> parseMerges(final String merges) { + if (merges == null) { + return null; + } + try { + Map<TableId,MergeInfo> result = new HashMap<>(); + DataInputBuffer buffer = new DataInputBuffer(); + byte[] data = Base64.getDecoder().decode(merges); + buffer.reset(data, data.length); + while (buffer.available() > 0) { + MergeInfo mergeInfo = new MergeInfo(); + mergeInfo.readFields(buffer); + result.put(mergeInfo.extent.tableId(), mergeInfo); + } + return result; + } catch (Exception ex) { + throw new RuntimeException(ex); + } + } + + private static boolean shouldReturnDueToSplit(final TabletMetadata tm, + final long splitThreshold) { + return tm.getFilesMap().values().stream().map(DataFileValue::getSize) + .collect(Collectors.summarizingLong(Long::longValue)).getSum() > splitThreshold; + } + + private static boolean shouldReturnDueToLocation(final TabletMetadata tm, + final Set<TableId> onlineTables, final Set<TServerInstance> current, final boolean debug) { + // is the table supposed to be online or offline? + final boolean shouldBeOnline = onlineTables.contains(tm.getTableId()); + + if (debug) { + LOG.debug("{} is {}. Table is {}line. Tablet hosting goal is {}, hostingRequested: {}", + tm.getExtent(), getLocationState(current, tm), (shouldBeOnline ? "on" : "off"), + tm.getHostingGoal(), tm.getHostingRequested()); + } + switch (getLocationState(current, tm)) { + case ASSIGNED: + // we always want data about assigned tablets + return true; + case HOSTED: + if (!shouldBeOnline || tm.getHostingGoal() == TabletHostingGoal.NEVER + || (tm.getHostingGoal() == TabletHostingGoal.ONDEMAND && !tm.getHostingRequested())) { + return true; + } + break; + case ASSIGNED_TO_DEAD_SERVER: + return true; + case SUSPENDED: + case UNASSIGNED: + if (shouldBeOnline && (tm.getHostingGoal() == TabletHostingGoal.ALWAYS + || (tm.getHostingGoal() == TabletHostingGoal.ONDEMAND && tm.getHostingRequested()))) { + return true; + } + break; + default: + throw new AssertionError( + "Inconceivable! The tablet is an unrecognized state: " + getLocationState(current, tm)); + } + return false; + } + + private static TabletState getLocationState(final Set<TServerInstance> liveServers, + final TabletMetadata tm) { + Location loc = tm.getLocation(); + + if (loc == null || loc.getType() == null || loc.getServerInstance() == null) { + return TabletState.UNASSIGNED; + } + + if (loc.getType().equals(LocationType.FUTURE)) { + return liveServers.contains(loc.getServerInstance()) ? TabletState.ASSIGNED + : TabletState.ASSIGNED_TO_DEAD_SERVER; + } else if (loc.getType().equals(LocationType.CURRENT)) { + return liveServers.contains(loc.getServerInstance()) ? TabletState.HOSTED + : TabletState.ASSIGNED_TO_DEAD_SERVER; + } else if (tm.getSuspend() != null) { + return TabletState.SUSPENDED; + } else { + return TabletState.UNASSIGNED; + } + } + + public static void configureScanner(final ScannerBase scanner, final CurrentState state) { + TabletColumnFamily.PREV_ROW_COLUMN.fetch(scanner); + scanner.fetchColumnFamily(CurrentLocationColumnFamily.NAME); + scanner.fetchColumnFamily(FutureLocationColumnFamily.NAME); + scanner.fetchColumnFamily(LastLocationColumnFamily.NAME); + scanner.fetchColumnFamily(SuspendLocationColumn.SUSPEND_COLUMN.getColumnFamily()); + scanner.fetchColumnFamily(LogColumnFamily.NAME); + scanner.fetchColumnFamily(ChoppedColumnFamily.NAME); + scanner.fetchColumnFamily(HostingColumnFamily.NAME); + scanner.addScanIterator(new IteratorSetting(1000, "wholeRows", WholeRowIterator.class)); + IteratorSetting tabletChange = + new IteratorSetting(1001, "ManagerTabletInfoIterator", ManagerTabletInfoIterator.class); + if (state != null) { + ManagerTabletInfoIterator.setCurrentServers(tabletChange, state.onlineTabletServers()); + ManagerTabletInfoIterator.setOnlineTables(tabletChange, state.onlineTables()); + ManagerTabletInfoIterator.setMerges(tabletChange, state.merges()); + // ELASTICITY_TODO passing the unassignemnt request as part of the migrations is a hack. Was + // not sure of the entire unassignment request approach and did not want to push it further + // into the code. + ManagerTabletInfoIterator.setMigrations(tabletChange, + Sets.union(state.migrationsSnapshot(), state.getUnassignmentRequest())); + ManagerTabletInfoIterator.setManagerState(tabletChange, state.getManagerState()); + ManagerTabletInfoIterator.setShuttingDown(tabletChange, state.shutdownServers()); + } + scanner.addScanIterator(tabletChange); + } + + public static ManagerTabletInfo decode(Entry<Key,Value> e) throws IOException { + return new ManagerTabletInfo(e.getKey(), e.getValue()); + } + + private Set<TServerInstance> current; + private Set<TableId> onlineTables; + private Map<TableId,MergeInfo> merges; + private boolean debug = false; + private Set<KeyExtent> migrations; + private ManagerState managerState = ManagerState.NORMAL; + private IteratorEnvironment env; + private Key topKey = null; + private Value topValue = null; + + @Override + public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options, + IteratorEnvironment env) throws IOException { + super.init(source, options, env); + this.env = env; + current = parseServers(options.get(SERVERS_OPTION)); + onlineTables = parseTableIDs(options.get(TABLES_OPTION)); + merges = parseMerges(options.get(MERGES_OPTION)); + debug = options.containsKey(DEBUG_OPTION); + migrations = parseMigrations(options.get(MIGRATIONS_OPTION)); + String managerStateOptionValue = options.get(MANAGER_STATE_OPTION); + try { + managerState = ManagerState.valueOf(managerStateOptionValue); + } catch (RuntimeException ex) { + if (managerStateOptionValue != null) { + LOG.error("Unable to decode managerState {}", managerStateOptionValue); + } + } + Set<TServerInstance> shuttingDown = parseServers(options.get(SHUTTING_DOWN_OPTION)); + if (current != null && shuttingDown != null) { + current.removeAll(shuttingDown); + } + } + + @Override + public Key getTopKey() { + return topKey; + } + + @Override + public Value getTopValue() { + return topValue; + } + + @Override + public boolean hasTop() { + return topKey != null && topValue != null; + } + + @Override + public void next() throws IOException { + topKey = null; + topValue = null; + super.next(); + } + + @Override + public void seek(Range range, Collection<ByteSequence> columnFamilies, boolean inclusive) + throws IOException { + topKey = null; + topValue = null; + super.seek(range, columnFamilies, inclusive); + } + + @Override + protected void consume() throws IOException { + + final Set<ManagementAction> reasonsToReturnThisTablet = new HashSet<>(); Review Comment: Could rename this to align with the new type name. ```suggestion final Set<ManagementAction> actions= new HashSet<>(); ``` ########## core/src/main/java/org/apache/accumulo/core/metadata/schema/RootTabletMetadata.java: ########## @@ -156,10 +159,11 @@ public SortedMap<Key,Value> toKeyValues() { * Convert this class to a {@link TabletMetadata} */ public TabletMetadata toTabletMetadata() { - // use a stream so we don't have to re-sort in a new TreeMap<Key,Value> structure - Stream<SimpleImmutableEntry<Key,Value>> entries = getKeyValues(); - return TabletMetadata.convertRow(entries.iterator(), - EnumSet.allOf(TabletMetadata.ColumnType.class), false); + TreeMap<Key,Value> entries = new TreeMap<>(); + getKeyValues().forEach(entry -> entries.put(entry.getKey(), entry.getValue())); + ManagerTabletInfo.addActions(entries, Set.of(ManagementAction.NEEDS_LOCATION_UPDATE)); Review Comment: Is this change still needed? ########## core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java: ########## @@ -116,7 +116,7 @@ public void testAllColumns() { SortedMap<Key,Value> rowMap = toRowMap(mutation); TabletMetadata tm = TabletMetadata.convertRow(rowMap.entrySet().iterator(), - EnumSet.allOf(ColumnType.class), true); + EnumSet.allOf(ColumnType.class), true, false); Review Comment: If not covered elsewhere, a unit test for the error suppression case would be nice. ########## server/manager/src/main/java/org/apache/accumulo/manager/Manager.java: ########## @@ -635,19 +635,19 @@ public TUnloadTabletGoal howUnload() { } } - TabletGoalState getSystemGoalState(TabletLocationState tls) { + TabletGoalState getSystemGoalState(TabletMetadata tm) { Review Comment: Looking though these changes it nice to see that TabletMetadata was an easy replacement. ########## core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java: ########## @@ -505,10 +529,24 @@ public static <E extends Entry<Key,Value>> TabletMetadata convertRow(Iterator<E> BulkFileColumnFamily.getBulkLoadTid(val)); break; case CurrentLocationColumnFamily.STR_NAME: - te.setLocationOnce(val, qual, LocationType.CURRENT); + try { + te.setLocationOnce(val, qual, LocationType.CURRENT); + } catch (IllegalStateException e) { + te.futureAndCurrentLocationSet = true; + if (!suppressLocationError) { + throw e; + } + } Review Comment: May be able to push checking `suppressLocationError` and setting `suppressLocationError` into `setLocationOnce` and can avoid catching the exception here. Something like the following. ```java private void setLocationOnce(String val, String qual, LocationType lt, boolean suppressError) { if (location != null) { if(suppressError) { futureAndCurrentLocationSet = true; } else { throw new IllegalStateException("Attempted to set second location for tableId: " + tableId + " endrow: " + endRow + " -- " + location + " " + qual + " " + val); } } location = new Location(val, qual, lt); } ``` ########## server/manager/src/main/java/org/apache/accumulo/manager/tableOps/delete/CleanUp.java: ########## @@ -91,18 +92,18 @@ public long isReady(long tid, Manager manager) throws Exception { boolean done = true; Range tableRange = new KeyExtent(tableId, null, null).toMetaRange(); Scanner scanner = manager.getContext().createScanner(MetadataTable.NAME, Authorizations.EMPTY); - MetaDataTableScanner.configureScanner(scanner, manager); + ManagerTabletInfoIterator.configureScanner(scanner, manager); Review Comment: This would be a follow on. As this iterator is intended to find tablets that need work, not sure its suitable for this case of deleting a table. We could optimize the iterator for this case to avoid looking for things that need splits for example, but not sure if that is worthwhile in terms of code complexity increase. As long as no harm comes from doing the unnecessary checks, maybe using the ManagerTabletInfoIterator here is fine. ########## server/tserver/src/main/java/org/apache/accumulo/tserver/UnloadTabletHandler.java: ########## @@ -114,19 +114,14 @@ public void run() { try { TServerInstance instance = new TServerInstance(server.clientAddress, server.getLock().getSessionId()); - TabletLocationState tls = null; - try { - tls = new TabletLocationState(extent, null, Location.current(instance), null, null, null, - false, TabletHostingGoal.ONDEMAND, false); - } catch (BadLocationStateException e) { - log.error("Unexpected error", e); - } + TabletMetadata tm = new TabletMetadataImposter(extent, null, Location.current(instance), null, Review Comment: We may want to open up a follow on issue here and add a TODO. Eventually the tablet may have a cahced version of a tabletmetadata obj it could pass here. Not sure though. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
