Skip to content

RATIS-2225. RaftClientRequest leak in RaftServerImpl.#1198

Merged
adoroszlai merged 1 commit intoapache:masterfrom
szetszwo:RATIS-2225
Dec 23, 2024
Merged

RATIS-2225. RaftClientRequest leak in RaftServerImpl.#1198
adoroszlai merged 1 commit intoapache:masterfrom
szetszwo:RATIS-2225

Conversation

@szetszwo
Copy link
Copy Markdown
Contributor

@szetszwo szetszwo commented Dec 23, 2024

See RATIS-2225

//RaftServerImpl.submitClientRequestAsync
    try {
      assertLifeCycleState(LifeCycle.States.RUNNING);
    } catch (ServerNotReadyException e) {
      final RaftClientReply reply = newExceptionReply(request, e);
      requestRef.release();
      return CompletableFuture.completedFuture(reply);
    }

In the code above,

  • it may catch ServerNotReadyException when the server is shutting down.
  • It calls newExceptionReply which may throw another exception.

Then, requestRef.release() is not called.

Also, when a unchecked exception is thrown, it won't be logged.

adoroszlai
adoroszlai approved these changes Dec 23, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @szetszwo for the fix, LGTM.

Currently running 10x10 test.

@szetszwo
Copy link
Copy Markdown
Contributor Author

szetszwo commented Dec 23, 2024

Currently running 10x10 test.

I ran it earlier. This change cannot completely fix the problem. We also need RATIS-2184. Then, it seems very close.

@adoroszlai adoroszlai merged commit 0514e09 into apache:master Dec 23, 2024
@szetszwo
Copy link
Copy Markdown
Contributor Author

szetszwo commented Dec 23, 2024

@adoroszlai , thanks a lot for reviewing and merging this!

@szetszwo
Copy link
Copy Markdown
Contributor Author

szetszwo commented Dec 23, 2024

Master after merged both RATIS-2225 and RATIS-2184: Total failures 3 https://github.com/apache/ratis/actions/runs/12472282367/job/34811870836

slfan1989 pushed a commit to slfan1989/ratis that referenced this pull request May 6, 2026
@szetszwo @slfan1989
RATIS-2225. RaftClientRequest leak in RaftServerImpl. (apache#1198)
50e2d7d 
(cherry picked from commit 0514e09)
@slfan1989 slfan1989 mentioned this pull request May 7, 2026
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adoroszlai adoroszlai adoroszlai approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@szetszwo @adoroszlai