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 2024/10/17 11:15:01 UTC
[PR] IGNITE-23314 PartitionReplicaListener#ensureReplicaIsPrimary does not require to use hybrid timestamp [ignite-3]
vldpyatkov opened a new pull request, #4581:
URL: https://github.com/apache/ignite-3/pull/4581
https://issues.apache.org/jira/browse/IGNITE-23314
--
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-23314 PartitionReplicaListener#ensureReplicaIsPrimary does not require to use hybrid timestamp [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #4581:
URL: https://github.com/apache/ignite-3/pull/4581#discussion_r1809043452
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/TestHybridClock.java:
##########
@@ -81,6 +81,13 @@ public long nowLong() {
}
}
+ @Override
+ public long currentLong() {
+ long now = currentTime();
Review Comment:
and rename now to current here.
--
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-23314 PartitionReplicaListener#ensureReplicaIsPrimary does not require to use hybrid timestamp [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #4581:
URL: https://github.com/apache/ignite-3/pull/4581#discussion_r1808256131
##########
modules/core/src/main/java/org/apache/ignite/internal/hlc/ClockService.java:
##########
@@ -31,13 +31,29 @@ public interface ClockService {
*/
HybridTimestamp now();
+ /**
+ * Gets a current timestamp. It is a fast way to get timestamp because it does not have to update internal state.
Review Comment:
I'd rather say "doesn't have to tick the logical part of the clock". Within the context of hybric clock that should be more clear. Here and in other comments.
##########
modules/core/src/main/java/org/apache/ignite/internal/hlc/HybridClockImpl.java:
##########
@@ -85,6 +85,13 @@ public long nowLong() {
}
}
+ @Override
+ public long currentLong() {
+ long now = currentTime();
Review Comment:
Let's use "current" here in order not to mess with "now" where "now" is unique.
--
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-23314 PartitionReplicaListener#ensureReplicaIsPrimary does not require to use hybrid timestamp [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #4581:
URL: https://github.com/apache/ignite-3/pull/4581#discussion_r1809044209
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java:
##########
@@ -3520,7 +3520,7 @@ private CompletableFuture<IgniteBiTuple<RowId, Collection<Lock>>> takeLocksForRe
* lease start time is not {@code null} in case of {@link PrimaryReplicaRequest}.
*/
private CompletableFuture<IgniteBiTuple<Boolean, Long>> ensureReplicaIsPrimary(ReplicaRequest request) {
- HybridTimestamp now = clockService.now();
+ HybridTimestamp now = clockService.current();
Review Comment:
and here: now -> current.
--
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-23314 PartitionReplicaListener#ensureReplicaIsPrimary does not require to use hybrid timestamp [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #4581:
URL: https://github.com/apache/ignite-3/pull/4581#discussion_r1809042343
##########
modules/core/src/main/java/org/apache/ignite/internal/hlc/HybridClock.java:
##########
@@ -28,13 +28,29 @@ public interface HybridClock {
*/
long nowLong();
+ /**
+ * Gets a current timestamp as long. It is a fast way to get timestamp because it does not have to update internal state.
Review Comment:
Could you please also fix the javadoc here.
##########
modules/core/src/main/java/org/apache/ignite/internal/hlc/HybridClock.java:
##########
@@ -28,13 +28,29 @@ public interface HybridClock {
*/
long nowLong();
+ /**
+ * Gets a current timestamp as long. It is a fast way to get timestamp because it does not have to update internal state.
+ * This timestamp is not unique, and equal to or less than that value is returned by {@link this#nowLong()}.
+ *
+ * @return The hybrid timestamp.
+ */
+ long currentLong();
+
/**
* Creates a timestamp for new event.
*
* @return The hybrid timestamp.
*/
HybridTimestamp now();
+ /**
+ * Gets a current timestamp. It is a fast way to get timestamp because it does not have to update internal state.
Review Comment:
And here.
--
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-23314 PartitionReplicaListener#ensureReplicaIsPrimary does not require to use hybrid timestamp [ignite-3]
Posted by "vldpyatkov (via GitHub)" <gi...@apache.org>.
vldpyatkov merged PR #4581:
URL: https://github.com/apache/ignite-3/pull/4581
--
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