[KYUUBI #5832] Always perform closing action in OperationLog to avoid fd leak

[Flyangz]

🔍 Description

Issue References 🔗

This pull request fixes #5832

Describe Your Solution 🔧

We should always close writer regardless of whether OperationLog is initialized or not, to prevent other threads from continuing to write.

Types of changes 🔖

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests

---

Checklists

📝 Author Self Checklist

• My code follows the style guidelines of this project
• I have performed a self-review
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• This patch was not authored or co-authored using Generative Tooling

📝 Committer Pre-Merge Checklist

• Pull request title is okay.
• No license issues.
• Milestone correctly set?
• Test coverage is ok
• Assignees are selected.
• Minimum number of approvals
• No changes are requested

Be nice. Be informative.

[Flyangz]

@pan3793 Would you mind reviewing this pr or do you know who can help?

[bowenliang123]

LGTM overall.
Could you attach some screenshots as evidence showing the fd leaks properly fixed by this PR? Eg. comparing opened fd numbers, or the server logs.

[pan3793]

would you like to try 1.7.3 or 1.8.0? this issue looks like was solved by #5211

[Flyangz]

would you like to try 1.7.3 or 1.8.0? this issue looks like was solved by #5211

@pan3793 My online test in issue #5832 had already included this #5211 patch, but the operation logs still leaked.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4c029f9) 61.23% compared to head (2549928) 61.21%.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #5854      +/-   ##
============================================
- Coverage     61.23%   61.21%   -0.03%     
  Complexity       23       23              
============================================
  Files           609      609              
  Lines         36281    36280       -1     
  Branches       4976     4976              
============================================
- Hits          22217    22207      -10     
- Misses        11669    11673       +4     
- Partials       2395     2400       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pan3793]

I see your point, the change makes sense to me. would be better to attach your production testing report to prove the patch takes effect as @bowenliang123 suggested.

[Flyangz]

Ok, I'll provide it as soon as possible. But I am busy with another more important project recently, and the upgrade of Kyuubi has been stoped. It will probably take a week or two before I can get back to it.

[bowenliang123]

Looking forward to your feedback and verification.

[pan3793]

Anyway, this fix looks good, let me merge it first, looking forward to your testing result once you have time @Flyangz

[pan3793]

Merged to master/1.8/1.7