vldpyatkov commented on code in PR #4700:
URL: https://github.com/apache/ignite-3/pull/4700#discussion_r1848122814
##########
modules/core/src/main/java/org/apache/ignite/internal/hlc/HybridClockImpl.java:
##########
@@ -107,33 +95,34 @@ private void notifyUpdateListeners(long newTs) {
}
@Override
- public HybridTimestamp now() {
+ public final HybridTimestamp now() {
return hybridTimestamp(nowLong());
}
@Override
- public HybridTimestamp current() {
+ public final HybridTimestamp current() {
return hybridTimestamp(currentLong());
}
/**
- * Updates the clock in accordance with an external event timestamp. If
the supplied timestamp is ahead of the
- * current clock timestamp, the clock gets adjusted to make sure it never
returns any timestamp before (or equal to)
- * the supplied external timestamp.
+ * Updates the clock in accordance with an external event timestamp. If
the supplied timestamp is ahead of the current clock timestamp,
+ * the clock gets adjusted to make sure it never returns any timestamp
before (or equal to) the supplied external timestamp.
*
* @param requestTime Timestamp from request.
* @return The resulting timestamp (guaranteed to exceed both previous
clock 'currentTs' and the supplied external ts).
*/
@Override
- public HybridTimestamp update(HybridTimestamp requestTime) {
+ public final HybridTimestamp update(HybridTimestamp requestTime) {
while (true) {
- long now = currentTime();
+ long now = physicalTime() << LOGICAL_TIME_BITS_SIZE;
// Read the latest time after accessing UTC time to reduce
contention.
- long oldLatestTime = this.latestTime;
+ long oldLatestTime = latestTime;
long newLatestTime = max(requestTime.longValue() + 1, max(now,
oldLatestTime + 1));
+ // TODO avoid CAS on lower value.
Review Comment:
I would recommend rewriting this method in this way (without TODO). It can
skip CAS in cases where the letest clock value is the most one and does not
notify a listener in case where the clock is chaged without jump up.
@rpuch Could you approve this approach with the listener?
```
public HybridTimestamp update(HybridTimestamp requestTime) {
long requestTimeLong = requestTime.longValue();
while (true) {
long now = currentTime();
// Read the latest time after accessing UTC time to reduce
contention.
long oldLatestTime = this.latestTime;
if (oldLatestTime >= requestTimeLong && oldLatestTime >= now) {
return hybridTimestamp(LATEST_TIME.incrementAndGet(this));
}
if (now > requestTimeLong) {
if (LATEST_TIME.compareAndSet(this, oldLatestTime, now)) {
return hybridTimestamp(now);
}
} else {
long newLatestTime = requestTimeLong + 1;
if (LATEST_TIME.compareAndSet(this, oldLatestTime,
newLatestTime)) {
notifyUpdateListeners(newLatestTime);
return hybridTimestamp(newLatestTime);
}
}
}
}
```
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
