You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "ifesdjeen (via GitHub)" <gi...@apache.org> on 2025/07/18 13:15:03 UTC
[PR] Improve gc rebased [cassandra-accord]
ifesdjeen opened a new pull request, #225:
URL: https://github.com/apache/cassandra-accord/pull/225
(had to re-post the patch since turned out I did merge Accord side yesterday)
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Improve gc rebased [cassandra-accord]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on PR #225:
URL: https://github.com/apache/cassandra-accord/pull/225#issuecomment-3094371314
@belliottsmith could you review?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Improve gc rebased [cassandra-accord]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen closed pull request #225: Improve gc rebased
URL: https://github.com/apache/cassandra-accord/pull/225
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Improve gc rebased [cassandra-accord]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on code in PR #225:
URL: https://github.com/apache/cassandra-accord/pull/225#discussion_r2216027280
##########
accord-core/src/main/java/accord/utils/PersistentField.java:
##########
@@ -92,36 +92,43 @@ public synchronized AsyncResult<?> mergeAndUpdate(@Nonnull Input inputValue)
private AsyncResult<?> mergeAndUpdate(@Nullable Input inputValue, @Nonnull BiFunction<Input, Saved, Saved> merge)
{
Invariants.nonNull(merge, "merge cannot be null");
- AsyncResult.Settable<Void> result = AsyncResults.settable();
Saved startingValue = latestPending;
if (startingValue == null)
{
Invariants.require(pending.isEmpty());
startingValue = currentValue.get();
}
Saved newValue = merge.apply(inputValue, startingValue);
- this.latestPending = newValue;
- int id = ++nextId;
- pending.add(new Pending<>(id, newValue));
- AsyncResult<?> pendingWrite = persister.persist(inputValue, newValue);
- pendingWrite.invoke((success, fail) -> {
- synchronized (this)
- {
- complete.add(id);
- boolean upd = false;
- Saved latest = null;
- while (!complete.isEmpty() && pending.peek().id == complete.first())
+ if (persister.shouldPersist(inputValue, newValue))
+ {
+ this.latestPending = newValue;
+ int id = ++nextId;
+ pending.add(new Pending<>(id, newValue));
+
+ AsyncResult.Settable<Void> result = AsyncResults.settable();
+ AsyncResult<?> pendingWrite = persister.persist(inputValue, newValue);
+ pendingWrite.invoke((success, fail) -> {
+ synchronized (this)
{
- latest = pending.poll().saving;
- complete.pollFirst();
- upd = true;
+ complete.add(id);
+ boolean upd = false;
+ Saved latest = null;
+ while (!complete.isEmpty() && pending.peek().id == complete.first())
+ {
+ latest = pending.poll().saving;
+ complete.pollFirst();
+ upd = true;
+ }
+ if (upd) set.accept(latest);
}
- if (upd) set.accept(latest);
result.setSuccess(null);
Review Comment:
here, the "biggest" change is that we set success outside synchronized block to avoid callbacks being called in it by accident
##########
accord-core/src/main/java/accord/utils/PersistentField.java:
##########
@@ -104,24 +103,31 @@ private AsyncResult<?> mergeAndUpdate(@Nullable Input inputValue, @Nonnull BiFun
int id = ++nextId;
pending.add(new Pending<>(id, newValue));
- AsyncResult<?> pendingWrite = persister.persist(inputValue, newValue);
- pendingWrite.invoke((success, fail) -> {
- synchronized (this)
- {
- complete.add(id);
- boolean upd = false;
- Saved latest = null;
- while (!complete.isEmpty() && pending.peek().id == complete.first())
+ if (persister.shouldPersist(inputValue, newValue))
+ {
+ AsyncResult.Settable<Void> result = AsyncResults.settable();
+ AsyncResult<?> pendingWrite = persister.persist(inputValue, newValue);
+ pendingWrite.invoke((success, fail) -> {
+ synchronized (this)
{
- latest = pending.poll().saving;
- complete.pollFirst();
- upd = true;
+ complete.add(id);
+ boolean upd = false;
+ Saved latest = null;
+ while (!complete.isEmpty() && pending.peek().id == complete.first())
+ {
+ latest = pending.poll().saving;
+ complete.pollFirst();
+ upd = true;
+ }
+ if (upd) set.accept(latest);
+ result.setSuccess(null);
Review Comment:
here, the "biggest" change is that we set success outside synchronized block to avoid callbacks being called there by accident
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Improve gc rebased [cassandra-accord]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on code in PR #225:
URL: https://github.com/apache/cassandra-accord/pull/225#discussion_r2216023231
##########
accord-core/src/main/java/accord/local/Node.java:
##########
@@ -310,6 +309,8 @@ public AsyncResult<?> persist(DurableBefore addValue, DurableBefore newValue)
Invariants.require(addValue.maxEpoch() <= epoch());
return wrap.persist(addValue, newValue);
}
+
+ @Override public boolean shouldPersist(DurableBefore addValue, DurableBefore newValue) { return wrap.shouldPersist(addValue, newValue); }
Review Comment:
re-did the patch, changed it so that we use `shouldPersist` instead of abstract class
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org