cassandra-pr mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From belliottsmith <...@git.apache.org>
Subject [GitHub] cassandra pull request #267: Consolidate batch write code
Date Thu, 20 Sep 2018 12:14:53 GMT
Github user belliottsmith commented on a diff in the pull request:

    https://github.com/apache/cassandra/pull/267#discussion_r219139414
  
    --- Diff: src/java/org/apache/cassandra/locator/ReplicaPlans.java ---
    @@ -61,26 +73,62 @@
             return forSingleReplicaWrite(keyspace, token, replica);
         }
     
    +    public static ReplicaPlan.ForTokenWrite forLocalBatchlogWrite()
    +    {
    +        Token token = DatabaseDescriptor.getPartitioner().getMinimumToken();
    +        Keyspace systemKeypsace = Keyspace.open(SchemaConstants.SYSTEM_KEYSPACE_NAME);
    +        Replica localSystemReplica = SystemReplicas.getSystemReplica(FBUtilities.getBroadcastAddressAndPort());
    +
    +        ReplicaLayout.ForTokenWrite liveAndDown = ReplicaLayout.forTokenWrite(
    +        EndpointsForToken.of(token, localSystemReplica),
    +        EndpointsForToken.empty(token)
    +        );
    +
    +        return forWrite(systemKeypsace, ConsistencyLevel.ONE, liveAndDown, liveAndDown,
writeAll);
    +    }
    +
         /**
          * Requires that the provided endpoints are alive.  Converts them to their relevant
system replicas.
          * Note that the liveAndDown collection and live are equal to the provided endpoints.
    -     *
    -     * The semantics are a bit weird, in that CL=ONE iff we have one node provided, and
otherwise is equal to TWO.
    -     * How these CL were chosen, and why we drop the CL if only one live node is available,
are both unclear.
          */
    -    public static ReplicaPlan.ForTokenWrite forBatchlogWrite(Keyspace keyspace, Collection<InetAddressAndPort>
endpoints) throws UnavailableException
    +    public static ReplicaPlan.ForTokenWrite forBatchlogWrite(String localDataCenter,
ConsistencyLevel consistencyLevel) throws UnavailableException
    --- End diff --
    
    Hmm.  It looks like `consistencyLevel` is almost completely ignored, besides for `==CL.ANY`,
but callers seem to then use the provided `consistencyLevel` to construct an `AbstractWriteResponseHandler`.
 
    
    Should we really be accepting a consistencyLevel parameter here?  All we really need to
know is if the caller needs durability (i.e. if `CL.ANY` was provided), and perhaps we should
have a separate enum (or just a boolean) for this, as it is otherwise rather confusing.  Though
what `CL.ANY` even means for batch log, I haven't a clue, anyway.
    
    Either way, while we're here we should document in the caller the fact that the provided
`consistencyLevel` is not used, and that the `batchlogConsistencyLevel` is used to clear the
entries from the remote batch logs once the real write has reached that consistency level.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


Mime
View raw message