vldpyatkov commented on code in PR #1501:
URL: https://github.com/apache/ignite-3/pull/1501#discussion_r1091766819


##########
modules/table/src/main/java/org/apache/ignite/internal/table/InternalTable.java:
##########
@@ -374,6 +425,13 @@ Publisher<BinaryRow> lookup(
      */
     List<String> assignments();
 
+    /**
+     * Gets a list of current table leader assignments with raft term.
+     *
+     * @return List of current leader assignments with raft term.
+     */
+    List<IgniteBiTuple<String, Long>> leaderAssignmentsWithTerm();

Review Comment:
   Before you wrote: _ClusterNode leaderNode_, but here leader is represented 
by String. This is a confusion API.
   Better to use term PrimaryReplica (of course, this class need to be created 
before) to represent a tuple: ClusterNode and Long.



##########
modules/table/src/main/java/org/apache/ignite/internal/table/InternalTable.java:
##########
@@ -357,6 +385,29 @@ Publisher<BinaryRow> lookup(
             @Nullable BitSet columnsToInclude
     );
 
+    /**
+     * Lookup rows corresponding to the given key given partition index, 
providing {@link Publisher}
+     * that reactively notifies about partition rows.
+     *
+     * @param partId The partition.
+     * @param txId Transaction id.
+     * @param leaderNode Raft group leader node that must handle given get 
request.
+     * @param leaderTerm Raft group leader term.
+     * @param indexId Index id.
+     * @param key Key to search.
+     * @param columnsToInclude Row projection.
+     * @return {@link Publisher} that reactively notifies about partition rows.
+     */
+    Publisher<BinaryRow> lookup(
+            int partId,
+            UUID txId,
+            ClusterNode leaderNode,
+            long leaderTerm,

Review Comment:
   The words leader and term should not permeate to the interface. Because we 
have a plane to use different method for replication for user data.



##########
modules/table/src/main/java/org/apache/ignite/internal/table/InternalTable.java:
##########
@@ -296,6 +297,31 @@ Publisher<BinaryRow> scan(
             @Nullable BitSet columnsToInclude
     );
 
+    /**
+     * Scans given partition index, providing {@link Publisher} that 
reactively notifies about partition rows.
+     *
+     * @param partId The partition.
+     * @param txId Transaction id.
+     * @param leaderNode Raft group leader node that must handle given get 
request.
+     * @param leaderTerm Raft group leader term.
+     * @param lowerBound Lower search bound.
+     * @param upperBound Upper search bound.
+     * @param flags Control flags. See {@link 
org.apache.ignite.internal.storage.index.SortedIndexStorage} constants.
+     * @param columnsToInclude Row projection.
+     * @return {@link Publisher} that reactively notifies about partition rows.
+     */
+    Publisher<BinaryRow> scan(
+            int partId,
+            UUID txId,
+            ClusterNode leaderNode,
+            long leaderTerm,

Review Comment:
   Leader and term need to be wrapped to PrimaryReplica class.



##########
modules/table/src/main/java/org/apache/ignite/internal/table/InternalTable.java:
##########
@@ -374,6 +425,13 @@ Publisher<BinaryRow> lookup(
      */
     List<String> assignments();
 
+    /**
+     * Gets a list of current table leader assignments with raft term.
+     *
+     * @return List of current leader assignments with raft term.
+     */
+    List<IgniteBiTuple<String, Long>> leaderAssignmentsWithTerm();

Review Comment:
   Also, you have to understand Placement driver is not a static collection. It 
will be a service with a method to get a PrimaryReplica by ReplicationGroupId. 



-- 
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]

Reply via email to