dcapwell commented on code in PR #3662: URL: https://github.com/apache/cassandra/pull/3662#discussion_r1838600519
########## src/java/org/apache/cassandra/db/virtual/AccordDebugKeyspace.java: ########## @@ -0,0 +1,663 @@ +/* + * 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.db.virtual; + +import java.nio.ByteBuffer; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Sets; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import accord.api.RoutingKey; +import accord.impl.DurabilityScheduling; +import accord.impl.progresslog.DefaultProgressLog; +import accord.impl.progresslog.TxnStateKind; +import accord.local.CommandStores; +import accord.local.DurableBefore; +import accord.local.MaxConflicts; +import accord.local.Node; +import accord.local.RedundantBefore; +import accord.local.RejectBefore; +import accord.primitives.Routable; +import accord.primitives.Status; +import accord.primitives.Timestamp; +import accord.primitives.Txn; +import accord.primitives.TxnId; +import accord.utils.Invariants; +import accord.utils.async.AsyncChain; +import org.apache.cassandra.cql3.statements.schema.CreateTableStatement; +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.Keyspace; +import org.apache.cassandra.db.marshal.ByteBufferAccessor; +import org.apache.cassandra.db.marshal.Int32Type; +import org.apache.cassandra.db.marshal.LongType; +import org.apache.cassandra.db.marshal.TupleType; +import org.apache.cassandra.db.marshal.UTF8Type; +import org.apache.cassandra.db.marshal.UUIDType; +import org.apache.cassandra.dht.Range; +import org.apache.cassandra.dht.Token; +import org.apache.cassandra.exceptions.InvalidRequestException; +import org.apache.cassandra.schema.Schema; +import org.apache.cassandra.schema.TableId; +import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.service.accord.AccordCommandStore; +import org.apache.cassandra.service.accord.AccordKeyspace; +import org.apache.cassandra.service.accord.AccordService; +import org.apache.cassandra.service.accord.AccordStateCache; +import org.apache.cassandra.service.accord.CommandStoreTxnBlockedGraph; +import org.apache.cassandra.service.accord.api.AccordRoutingKey; +import org.apache.cassandra.service.accord.api.AccordRoutingKey.TokenKey; +import org.apache.cassandra.service.consensus.migration.ConsensusMigrationState; +import org.apache.cassandra.service.consensus.migration.TableMigrationState; +import org.apache.cassandra.tcm.ClusterMetadata; +import org.apache.cassandra.utils.ByteBufferUtil; +import org.apache.cassandra.utils.Pair; + +import static java.lang.String.format; +import static java.util.Comparator.comparing; +import static com.google.common.collect.ImmutableList.toImmutableList; +import static accord.utils.async.AsyncChains.getBlockingAndRethrow; +import static org.apache.cassandra.schema.SchemaConstants.VIRTUAL_ACCORD_DEBUG; +import static org.apache.cassandra.utils.ByteBufferUtil.bytes; + +public class AccordDebugKeyspace extends VirtualKeyspace Review Comment: > The coordination table as discussed already I consider to be pretty useless, stated in another comment, I am fine saying we will punt on this table closer to the release and drop it for now. What is running is very useful but if you both don't feel we are ready for that we can drop for now. > The txn block table... But this leaks at least: SaveStatus, ExecuteAt, StoreId, but also leaks details about how we manage execution dependencies (which has already been drastically modified more than once during development). this table is critical for someone to answer the question "why did my txn timeout", as it shows what is blocking it still. If we say key vs txn dependencies should not be exposed then thats mostly fine as the txn_id matters the most... if you are blocked on X you then can ask why is X blocked and keep going until you find what is stuck. But if you don't expose save status and timestamps then what can you meaningfully say why something is stuck? So, if you prefer to drop the key vs txn part, I am fine removing in this patch. If you feel the txn id and save status is too much, then we have problems as I don't know how to answer the above question. > I did not anyway honestly consider this was intended to be consumed by applications, nor was I aware of any controversy about human-only virtual tables. when ever there has been a patch to alter any existing table the same conversation comes up about backwards compatibility; once it reaches the public API we have to maintain it. We have 0 tables where this stance doesn't hold true today, this patch is arguing to be the first. As stated in the other thread, I am fine with a view point that these tables are for testing so off by default and off in prod, but exist to aid developers writing patches or to improve test errors; there is clear value in that. -- 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]
