You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "Cyrill (via GitHub)" <gi...@apache.org> on 2024/01/29 12:33:56 UTC
[PR] IGNITE-20377 Rename term to enlistmentConsistencyToken [ignite-3]
Cyrill opened a new pull request, #3110:
URL: https://github.com/apache/ignite-3/pull/3110
https://issues.apache.org/jira/browse/IGNITE-20377
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20377 Rename term to enlistmentConsistencyToken [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #3110:
URL: https://github.com/apache/ignite-3/pull/3110#discussion_r1471644276
##########
modules/client/src/test/java/org/apache/ignite/client/fakes/FakeTxManager.java:
##########
@@ -84,7 +84,7 @@ public UUID id() {
}
@Override
- public IgniteBiTuple<ClusterNode, Long> enlistedNodeAndTerm(TablePartitionId tablePartitionId) {
Review Comment:
Please remove // TODO: https://issues.apache.org/jira/browse/IGNITE-20377 in PartitionReplicaListener.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java:
##########
@@ -83,7 +83,7 @@
import org.apache.ignite.internal.sql.engine.exec.ExecutionServiceImpl;
import org.apache.ignite.internal.sql.engine.exec.LifecycleAware;
import org.apache.ignite.internal.sql.engine.exec.MailboxRegistryImpl;
-import org.apache.ignite.internal.sql.engine.exec.NodeWithTerm;
+import org.apache.ignite.internal.sql.engine.exec.NodeWithEnlistmentToken;
Review Comment:
In any case I'd use consistent naming pattern everywhere it's with EnlistementToken or just Token but not mixing of nodeAndToken, nodeWithEnlistmentToken, etc.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ScannableTable.java:
##########
@@ -33,29 +33,29 @@ public interface ScannableTable {
* Performs a scan over table.
*
* @param ctx Execution context.
- * @param partWithTerm Partition.
+ * @param partWithToken Partition.
* @param rowFactory Row factory.
* @param requiredColumns Required columns.
* @return A publisher that produces rows.
* @param <RowT> A type of row.
*/
- <RowT> Publisher<RowT> scan(ExecutionContext<RowT> ctx, PartitionWithTerm partWithTerm,
+ <RowT> Publisher<RowT> scan(ExecutionContext<RowT> ctx, PartitionWithEnlistmentToken partWithToken,
RowFactory<RowT> rowFactory, @Nullable BitSet requiredColumns);
/**
* Performs range scan using the given index.
*
* @param <RowT> A type of row.
* @param ctx Execution context.
- * @param partWithTerm Partition.
+ * @param partWithToken Partition.
Review Comment:
Same as above. Consistent naming. Won't repeat this below.
##########
modules/client/src/test/java/org/apache/ignite/client/fakes/FakeTxManager.java:
##########
@@ -84,7 +84,7 @@ public UUID id() {
}
@Override
- public IgniteBiTuple<ClusterNode, Long> enlistedNodeAndTerm(TablePartitionId tablePartitionId) {
+ public IgniteBiTuple<ClusterNode, Long> enlistedNodeAndToken(TablePartitionId tablePartitionId) {
Review Comment:
So, you believe that token instead of enlistmentConsistencyToken or consistencyToken is clear enough?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionServiceImpl.java:
##########
@@ -1022,17 +1022,20 @@ private void enlist(int tableId, List<NodeWithTerm> assignments) {
for (int p = 0; p < partsCnt; p++) {
TablePartitionId tablePartId = new TablePartitionId(tableId, p);
- NodeWithTerm enlistmentToken = assignments.get(p);
+ NodeWithEnlistmentToken enlistmentToken = assignments.get(p);
Review Comment:
Could you please also rename the `enlistmentToken` to node or similar? `enlistmentToken.enlistmentConsistencyToken()` looks confusing.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20377 Rename term to enlistmentConsistencyToken [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc merged PR #3110:
URL: https://github.com/apache/ignite-3/pull/3110
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org