Skip to content

cephfs-journal-tool: Journal trimming issue#65577

Merged
vshankar merged 2 commits intoceph:mainfrom
kotreshhr:tools-journal-trimming-issue
Sep 19, 2025
Merged

cephfs-journal-tool: Journal trimming issue#65577
vshankar merged 2 commits intoceph:mainfrom
kotreshhr:tools-journal-trimming-issue

Conversation

@kotreshhr
Copy link
Contributor

@kotreshhr kotreshhr commented Sep 18, 2025

cephfs-journal-tool: Don't reset the journal trim position

If the fs had to go through journal recovery and reset,
the cephfs-journal-tool resets the journal trim position
because of which the old unused journal objects just stay
forever in the metadata pool. The patch fixes the issue.
Now, the old stale journal objects are trimmed during the
regular trimming cycle helping to recover space in the
metadata pool.

Fixes: https://tracker.ceph.com/issues/69708
Signed-off-by: Kotresh HR khiremat@redhat.com

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

You must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.

@github-actions github-actions bot added cephfs Ceph File System tests labels Sep 18, 2025
@kotreshhr kotreshhr force-pushed the tools-journal-trimming-issue branch from 7e9735b to 27ad845 Compare September 18, 2025 08:00
@kotreshhr kotreshhr requested review from a team, batrick and vshankar September 18, 2025 08:24
@batrick
Copy link
Member

batrick commented Sep 18, 2025

flake8: install_deps /home/jenkins-build/build/workspace/ceph-pull-requests/qa> python -I -m pip install flake8
flake8: freeze /home/jenkins-build/build/workspace/ceph-pull-requests/qa> python -m pip freeze --all
flake8: flake8==7.3.0,mccabe==0.7.0,pip==25.2,pycodestyle==2.14.0,pyflakes==3.4.0,setuptools==80.9.0
flake8: commands[0] /home/jenkins-build/build/workspace/ceph-pull-requests/qa> flake8 --select=F,E9 --exclude=venv,.tox
./tasks/cephfs/test_journal_repair.py:156:16: F821 undefined name 're'
./tasks/cephfs/test_journal_repair.py:173:50: F821 undefined name 'StringIO'
./tasks/cephfs/test_journal_repair.py:187:9: F841 local variable 'status' is assigned to but never used
./tasks/cephfs/test_journal_repair.py:196:50: F821 undefined name 'StringIO'
flake8: exit 1 (6.61 seconds) /home/jenkins-build/build/workspace/ceph-pull-requests/qa> flake8 --select=F,E9 --exclude=venv,.tox pid=827211
flake8: FAIL ✖ in 10.55 seconds

@kotreshhr kotreshhr force-pushed the tools-journal-trimming-issue branch from 27ad845 to b710c07 Compare September 18, 2025 11:48
@kotreshhr
Copy link
Contributor Author

flake8: install_deps /home/jenkins-build/build/workspace/ceph-pull-requests/qa> python -I -m pip install flake8
flake8: freeze /home/jenkins-build/build/workspace/ceph-pull-requests/qa> python -m pip freeze --all
flake8: flake8==7.3.0,mccabe==0.7.0,pip==25.2,pycodestyle==2.14.0,pyflakes==3.4.0,setuptools==80.9.0
flake8: commands[0] /home/jenkins-build/build/workspace/ceph-pull-requests/qa> flake8 --select=F,E9 --exclude=venv,.tox
./tasks/cephfs/test_journal_repair.py:156:16: F821 undefined name 're'
./tasks/cephfs/test_journal_repair.py:173:50: F821 undefined name 'StringIO'
./tasks/cephfs/test_journal_repair.py:187:9: F841 local variable 'status' is assigned to but never used
./tasks/cephfs/test_journal_repair.py:196:50: F821 undefined name 'StringIO'
flake8: exit 1 (6.61 seconds) /home/jenkins-build/build/workspace/ceph-pull-requests/qa> flake8 --select=F,E9 --exclude=venv,.tox pid=827211
flake8: FAIL ✖ in 10.55 seconds

Thanks for the quick review. I will fix this up.

@kotreshhr kotreshhr force-pushed the tools-journal-trimming-issue branch from b710c07 to 5b698e7 Compare September 18, 2025 11:59
@kotreshhr kotreshhr requested a review from a team September 18, 2025 12:11
Validates that the cephfs-journal-tool reset
doesn't reset the trim position so that the
journal trim takes care of trimming the older
unused journal objects helping to recover the
space in metadata pool.

Fixes: https://tracker.ceph.com/issues/69708
Signed-off-by: Kotresh HR <khiremat@redhat.com>
If the fs had to go through journal recovery and reset,
the cephfs-journal-tool resets the journal trim position
because of which the old unused journal objects just stay
forever in the metadata pool. The patch fixes the issue.
Now, the old stale journal objects are trimmed during the
regular trimming cycle helping to recover space in the
metadata pool.

Fixes: https://tracker.ceph.com/issues/69708
Signed-off-by: Kotresh HR <khiremat@redhat.com>
@kotreshhr kotreshhr force-pushed the tools-journal-trimming-issue branch from 5b698e7 to 4f9a926 Compare September 18, 2025 18:12
@kotreshhr
Copy link
Contributor Author

QA Results validating the test added.

Test added fails without the fix against current main branch:
https://pulpito.ceph.com/khiremat-2025-09-18_18:19:08-fs:functional-main-distro-default-smithi/

Test passes with the fix:
https://pulpito.ceph.com/khiremat-2025-09-18_18:14:24-fs:functional-wip-khiremat-65577-debug-distro-default-smithi/

Copy link
Contributor

@vshankar vshankar left a comment

Choose a reason for hiding this comment

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

Nicely implemented 👍

@vshankar vshankar merged commit 15ebe51 into ceph:main Sep 19, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cephfs Ceph File System tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants