You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "vldpyatkov (via GitHub)" <gi...@apache.org> on 2023/04/17 12:14:48 UTC

[GitHub] [ignite-3] vldpyatkov opened a new pull request, #1946: IGNITE-18958 Implement handling of lease grant responses on placement…

vldpyatkov opened a new pull request, #1946:
URL: https://github.com/apache/ignite-3/pull/1946

   … driver side


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


[GitHub] [ignite-3] kgusakov commented on a diff in pull request #1946: IGNITE-18958 Implement handling of lease grant responses on placement…

Posted by "kgusakov (via GitHub)" <gi...@apache.org>.
kgusakov commented on code in PR #1946:
URL: https://github.com/apache/ignite-3/pull/1946#discussion_r1170166264


##########
modules/placement-driver/src/main/java/org/apache/ignite/internal/placementdriver/LeaseUpdater.java:
##########
@@ -210,16 +217,35 @@ public void run() {
 
                     Lease lease = leaseTracker.getLease(grpId);
 
+                    if (!lease.isAccepted()) {
+                        LeaseAgreement agreement = leaseNegotiator.negotiated(grpId);
+
+                        if (agreement.isAccepted()) {
+                            publishLease(grpId, lease);
+
+                            continue;
+                        } else if (agreement.ready()) {
+                            ClusterNode candidate = nextLeaseHolder(entry.getValue(), agreement.getRedirectTo());
+
+                            if (candidate == null) {
+                                continue;
+                            }
+
+                            // New lease is granting.
+                            writeNewLeasInMetaStorage(grpId, lease, candidate);

Review Comment:
   Not sure, that I understand: why don't we need to get acceptance from the `agreement.getRedirectTo` replica in this case?



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


[GitHub] [ignite-3] vldpyatkov merged pull request #1946: IGNITE-18958 Implement handling of lease grant responses on placement…

Posted by "vldpyatkov (via GitHub)" <gi...@apache.org>.
vldpyatkov merged PR #1946:
URL: https://github.com/apache/ignite-3/pull/1946


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


[GitHub] [ignite-3] vldpyatkov commented on a diff in pull request #1946: IGNITE-18958 Implement handling of lease grant responses on placement…

Posted by "vldpyatkov (via GitHub)" <gi...@apache.org>.
vldpyatkov commented on code in PR #1946:
URL: https://github.com/apache/ignite-3/pull/1946#discussion_r1170310120


##########
modules/placement-driver/src/main/java/org/apache/ignite/internal/placementdriver/LeaseUpdater.java:
##########
@@ -210,16 +217,35 @@ public void run() {
 
                     Lease lease = leaseTracker.getLease(grpId);
 
+                    if (!lease.isAccepted()) {
+                        LeaseAgreement agreement = leaseNegotiator.negotiated(grpId);
+
+                        if (agreement.isAccepted()) {
+                            publishLease(grpId, lease);
+
+                            continue;
+                        } else if (agreement.ready()) {
+                            ClusterNode candidate = nextLeaseHolder(entry.getValue(), agreement.getRedirectTo());
+
+                            if (candidate == null) {
+                                continue;
+                            }
+
+                            // New lease is granting.
+                            writeNewLeasInMetaStorage(grpId, lease, candidate);

Review Comment:
   New lease is written to MS as unacceptable. When the new lease wrote, a negotiation procedure is started.
   Only when the lease is accepted by replica side, it will be written to MS as an accepted lease.
   The only place where a lease can be accepted is Updater#publishLease.
   All of these steps are valid for a redirect proposal (agreement.getRedirectTo).



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


[GitHub] [ignite-3] kgusakov commented on a diff in pull request #1946: IGNITE-18958 Implement handling of lease grant responses on placement…

Posted by "kgusakov (via GitHub)" <gi...@apache.org>.
kgusakov commented on code in PR #1946:
URL: https://github.com/apache/ignite-3/pull/1946#discussion_r1170022645


##########
modules/placement-driver/src/main/java/org/apache/ignite/internal/placementdriver/LeaseUpdater.java:
##########
@@ -173,14 +175,19 @@ public void deactivate() {
      * Finds a node that can be the leaseholder.
      *
      * @param assignments Replication group assignment.
+     * @param proposed Proposed consistent id, found out of a lease negotiation. The parameter might be {@code null}.
      * @return Cluster node, or {@code null} if no node in assignments can be the leaseholder.
      */
-    private ClusterNode nextLeaseHolder(Set<Assignment> assignments) {
+    private ClusterNode nextLeaseHolder(Set<Assignment> assignments, @Nullable String proposed) {

Review Comment:
   `proposed` sounds like a boolean flag. Could we rename it to anything like `proposedConsistentId`?



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