bobhan1 opened a new pull request, #68926:
URL: https://github.com/apache/doris/pull/68926
### What problem does this PR solve?
Issue Number: N/A
Related PR: #3649 (the existing-log resume path)
Problem Summary:
When a BE restarts with `log_split_method=size` and existing INFO/WARNING
logs, Doris's glog patch opens each resumed file with `open()` and then opens
it again with `fopen()`. The first descriptor has no owner on the success path.
Rotation and shutdown close only the `FILE*`; once the file quota unlinks the
old log, the leaked descriptor keeps its disk blocks allocated until the BE
exits.
Use `fdopen(fd, "a")` in both the glog 0.4.0 and 0.6.0 patches so the stream
owns the existing append-only, close-on-exec descriptor. Initialize the saved
file length only after `fdopen()` succeeds, and close the descriptor on failure
before using the existing new-file path. Skip resume when the file list is
empty, including when a quota of one removes the last existing file.
Add a standalone regression test against the actual static glog library and
run it after the Linux third-party build. It covers INFO and WARNING logs,
append behavior, rotation using the saved file length, repeated quota deletion,
shutdown, empty directories, quota one, and injected `fdopen()` failure.
### Release note
Fix leaked file descriptors that could retain disk space for deleted BE logs
after restarting with size-based log splitting.
### Check List (For Author)
- Test
- [ ] Regression test
- [ ] Unit Test
- [x] Manual test (standalone third-party library regression; details
below)
- [ ] No need to test or manual test.
- Behavior changed:
- [ ] No.
- [x] Yes. Resumed log descriptors are released during rotation and
shutdown; an empty resume list and failed `fdopen()` use the normal new-file
path.
- Does this need documentation?
- [x] No.
- [ ] Yes.
Validation:
- Applied both updated patches to clean upstream source archives with `patch
--dry-run --fuzz=0`, then rebuilt and installed the static libraries in
isolated directories (0.4.0 using autotools, 0.6.0 using CMake; optional gflags
and libunwind disabled).
- Ran `TP_INSTALL_DIR=<rebuilt-glog-install> CXX=/usr/bin/c++ bash
thirdparty/test/glog-logfile-resume-test.sh` for each version: all four
scenarios passed on each version. Two active log descriptors, zero descriptors
for deleted logs through quota rotations, and zero log descriptors after
shutdown.
- The same test against the existing unmodified Doris glog failed with
`initial write: held=4 expected=2`, confirming that it detects the original
leak.
- `git diff --check`, Bash syntax, ShellCheck 0.8.0, shfmt 3.5.1, embedded
C++ formatting with clang-format 16, and workflow YAML validation passed.
- A full BE build, BE unit tests, and cluster regression were not run;
validation directly rebuilt and exercised the patched dependency.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
