azotcsit commented on a change in pull request #1232: URL: https://github.com/apache/cassandra/pull/1232#discussion_r720698776
########## File path: src/java/org/apache/cassandra/hints/PendingHintsInfo.java ########## @@ -0,0 +1,87 @@ +/* + * 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.cassandra.hints; + +import java.io.Serializable; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.UUID; + +import com.google.common.base.MoreObjects; + +public class PendingHintsInfo +{ + public static final String HOST_ID = "host_id"; + public static final String TOTAL_FILES = "total_files"; + public static final String OLDEST_TIMESTAMP = "oldest_timestamp"; + public static final String NEWEST_TIMESTAMP = "newest_timestamp"; + + public final UUID hostId; + public final int totalFiles; + public final long oldestTimestamp; + public final long newestTimestamp; + + public PendingHintsInfo(UUID hostId, int totalFiles, long oldestTimestamp, long newestTimestamp) + { + this.hostId = hostId; + this.totalFiles = totalFiles; + this.oldestTimestamp = oldestTimestamp; + this.newestTimestamp = newestTimestamp; + } + + public Map<String, String> asMap() + { + Map<String, String> ret = new HashMap<>(); + ret.put(HOST_ID, hostId.toString()); + ret.put(TOTAL_FILES, String.valueOf(totalFiles)); + ret.put(OLDEST_TIMESTAMP, String.valueOf(oldestTimestamp)); + ret.put(NEWEST_TIMESTAMP, String.valueOf(newestTimestamp)); + return ret; + } + + @Override + public boolean equals(Object o) Review comment: I don't feel we really need `equals` and `hashCode` method because `PendingHintsInfo`is not stored in any hash map/set. ########## File path: src/java/org/apache/cassandra/tools/nodetool/ListEndpointsPendingHints.java ########## @@ -0,0 +1,90 @@ +/* + * 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.cassandra.tools.nodetool; + +import java.net.UnknownHostException; +import java.text.SimpleDateFormat; +import java.util.Date; +import java.util.List; +import java.util.Map; + +import io.airlift.airline.Command; +import org.apache.cassandra.hints.PendingHintsInfo; +import org.apache.cassandra.locator.EndpointSnitchInfoMBean; +import org.apache.cassandra.locator.InetAddressAndPort; +import org.apache.cassandra.tools.NodeProbe; +import org.apache.cassandra.tools.NodeTool; +import org.apache.cassandra.tools.nodetool.formatter.TableBuilder; + +@Command(name = "listendpointspendinghints", description = "Print all the endpoints that this node has hints for") +public class ListEndpointsPendingHints extends NodeTool.NodeToolCmd +{ + @Override + public void execute(NodeProbe probe) + { + List<PendingHintsInfo> pendingHints = probe.listPendingHints(); + if(pendingHints.isEmpty()) + { + probe.output().out.println("This node does not have hints for other endpoints"); + } + else + { + Map<String, String> endpointMap = probe.getHostIdToEndpointWithPort(); + Map<String, String> simpleStates = probe.getSimpleStatesWithPort(); + EndpointSnitchInfoMBean epSnitchInfo = probe.getEndpointSnitchInfoProxy(); + + SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss,SSS"); Review comment: Thank for showing sample outputs, it is very useful! nit: does it make sense to have this output in the same order as the VT has? ########## File path: test/unit/org/apache/cassandra/hints/HintsStoreTest.java ########## @@ -137,6 +138,26 @@ public void testConcurrentDeleteExpiredHints() throws Exception { assertFalse("All hints files should be deleted", store.hasFiles()); } + @Test + public void testPendingHintsInfo() throws Exception + { + HintsStore store = HintsCatalog.load(directory, ImmutableMap.of()).get(hostId); + assertEquals(new PendingHintsInfo(store.hostId, 0, Long.MAX_VALUE, Long.MIN_VALUE), + store.getPendingHintsInfo()); + + final long t1 = System.currentTimeMillis(); + writeHints(directory, new HintsDescriptor(hostId, t1), 100, t1); + store = HintsCatalog.load(directory, ImmutableMap.of()).get(hostId); + assertEquals(new PendingHintsInfo(store.hostId, 1, t1, t1), + store.getPendingHintsInfo()); + final long t2 = t1 + 1; + writeHints(directory, new HintsDescriptor(hostId, t2), 100, t2); + store = HintsCatalog.load(directory, ImmutableMap.of()).get(hostId); + assertEquals(new PendingHintsInfo(store.hostId, 2, t1, t2), + store.getPendingHintsInfo()); + Review comment: nit: unnecessary empty line ########## File path: src/java/org/apache/cassandra/hints/HintsService.java ########## @@ -268,6 +269,31 @@ public synchronized void shutdownBlocking() throws ExecutionException, Interrupt bufferPool.close(); } + /** + * Returns all pending hints that this node has. + * + * @return a list of {@link PendingHintsInfo} + */ + public List<PendingHintsInfo> getPendingHintsInfo() + { + return catalog.stores() + .filter(HintsStore::hasFiles) + .map(HintsStore::getPendingHintsInfo) + .collect(Collectors.toList()); + } + + /** + * Returns all pending hints that this node has. + * + * @return a list of maps with endpoints' ids, total number of hint files, their oldest and newest timestamps. + */ + public List<Map<String, String>> getPendingHints() Review comment: not: I'd probably move `PendingHintsInfo.asMap()` logic here because it is not really supposed to be used somewhere else. ########## File path: test/unit/org/apache/cassandra/hints/HintsStoreTest.java ########## @@ -137,6 +138,26 @@ public void testConcurrentDeleteExpiredHints() throws Exception { assertFalse("All hints files should be deleted", store.hasFiles()); } + @Test + public void testPendingHintsInfo() throws Exception + { + HintsStore store = HintsCatalog.load(directory, ImmutableMap.of()).get(hostId); + assertEquals(new PendingHintsInfo(store.hostId, 0, Long.MAX_VALUE, Long.MIN_VALUE), + store.getPendingHintsInfo()); + + final long t1 = System.currentTimeMillis(); + writeHints(directory, new HintsDescriptor(hostId, t1), 100, t1); + store = HintsCatalog.load(directory, ImmutableMap.of()).get(hostId); Review comment: nit: Collections.emptyMap() and below too ########## File path: test/unit/org/apache/cassandra/hints/HintsStoreTest.java ########## @@ -137,6 +138,26 @@ public void testConcurrentDeleteExpiredHints() throws Exception { assertFalse("All hints files should be deleted", store.hasFiles()); } + @Test + public void testPendingHintsInfo() throws Exception + { + HintsStore store = HintsCatalog.load(directory, ImmutableMap.of()).get(hostId); + assertEquals(new PendingHintsInfo(store.hostId, 0, Long.MAX_VALUE, Long.MIN_VALUE), Review comment: I'm not sure whether "empty" pending hints infor should be like this or `null`. @ekaterinadimitrova2 please, share your thoughts. ########## File path: test/unit/org/apache/cassandra/hints/HintsStoreTest.java ########## @@ -137,6 +138,26 @@ public void testConcurrentDeleteExpiredHints() throws Exception { assertFalse("All hints files should be deleted", store.hasFiles()); } + @Test + public void testPendingHintsInfo() throws Exception + { + HintsStore store = HintsCatalog.load(directory, ImmutableMap.of()).get(hostId); + assertEquals(new PendingHintsInfo(store.hostId, 0, Long.MAX_VALUE, Long.MIN_VALUE), + store.getPendingHintsInfo()); + + final long t1 = System.currentTimeMillis(); Review comment: Please, use `Clock`, see https://issues.apache.org/jira/browse/CASSANDRA-16923 for details. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
