You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by "Elia-Renzoni (via GitHub)" <gi...@apache.org> on 2026/09/25 08:55:00 UTC
[PR] RATIS-2399. Introduce separate locks for commitIndex and snapshotIndex in RaftLogBase [ratis]
Elia-Renzoni opened a new pull request, #1612:
URL: https://github.com/apache/ratis/pull/1612
## What changes were proposed in this pull request?
Replace the single lock for both snapshotIndex and commitIndex in RaftLogBase with two separate locks.
## What is the link to the Apache JIRA
Jira Issue link:
https://issues.apache.org/jira/browse/RATIS-2399?page=com.atlassian.jira.plugin.system.issuetabpanels%3Aall-tabpanel
## How was this patch tested?
This patch was tested via unit test
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-2399. Introduce separate locks for commitIndex and snapshotIndex in RaftLogBase [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #1612:
URL: https://github.com/apache/ratis/pull/1612#discussion_r4171415310
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/RaftLogBase.java:
##########
@@ -153,7 +165,8 @@ protected void updateSnapshotIndexFromStateMachine() {
@Override
public void updateSnapshotIndex(long newSnapshotIndex) {
- try(AutoCloseableLock writeLock = writeLock()) {
+ try(AutoCloseableLock commitLock = writeLock(LockType.COMMIT_INDEX);
+ AutoCloseableLock snapshotLock = writeLock(LockType.SNAPSHOT_INDEX)) {
Review Comment:
@Elia-Renzoni , SNAPSHOT_INDEX is only acquired here together with COMMIT_INDEX. All the other cases acquire COMMIT_INDEX. Since it already has COMMIT_INDEX, acquiring SNAPSHOT_INDEX has no effect. No?
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-2399. Introduce separate locks for commitIndex and snapshotIndex in RaftLogBase [ratis]
Posted by "Elia-Renzoni (via GitHub)" <gi...@apache.org>.
Elia-Renzoni commented on code in PR #1612:
URL: https://github.com/apache/ratis/pull/1612#discussion_r4195273742
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/RaftLogBase.java:
##########
@@ -153,7 +165,8 @@ protected void updateSnapshotIndexFromStateMachine() {
@Override
public void updateSnapshotIndex(long newSnapshotIndex) {
- try(AutoCloseableLock writeLock = writeLock()) {
+ try(AutoCloseableLock commitLock = writeLock(LockType.COMMIT_INDEX);
+ AutoCloseableLock snapshotLock = writeLock(LockType.SNAPSHOT_INDEX)) {
Review Comment:
Yes, you're right. I've pushed commit fac0204dd16060a4d2520fd4f5d1e9dbfb24819a with the fix. However, I've changed the original scope of the PR quite a bit I had previously added two separate locks, but since snapshotIndexLock was effectively unused, I've gone back to a single lock as before and used updateToMax to update the index atomically. What do you think of the latest changes?
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-2399. Introduce separate locks for commitIndex and snapshotIndex in RaftLogBase [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #1612:
URL: https://github.com/apache/ratis/pull/1612#discussion_r4171415310
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/RaftLogBase.java:
##########
@@ -153,7 +165,8 @@ protected void updateSnapshotIndexFromStateMachine() {
@Override
public void updateSnapshotIndex(long newSnapshotIndex) {
- try(AutoCloseableLock writeLock = writeLock()) {
+ try(AutoCloseableLock commitLock = writeLock(LockType.COMMIT_INDEX);
+ AutoCloseableLock snapshotLock = writeLock(LockType.SNAPSHOT_INDEX)) {
Review Comment:
@Elia-Renzoni , SNAPSHOT_INDEX is only acquired here together with COMMIT_INDEX. All the other cases acquire COMMIT_INDEX. So, SNAPSHOT_INDEX has no effect. Now?
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org