Skip to content

crimson/os/seastore: cleanups related to managing omap and log tree #62079

Merged
cyx1231st merged 20 commits intoceph:mainfrom
myoungwon:seastore-omap-log-cleanup
Mar 17, 2025
Merged

crimson/os/seastore: cleanups related to managing omap and log tree #62079
cyx1231st merged 20 commits intoceph:mainfrom
myoungwon:seastore-omap-log-cleanup

Conversation

@myoungwon
Copy link
Member

@myoungwon myoungwon commented Mar 2, 2025 •
edited
Loading

Follow-up:
#59213 (comment)
#59213 (comment)

Signed-off-by: Yingxin Cheng yingxin.cheng@intel.com
Signed-off-by: Myoungwon Oh ohmyoungwon@gmail.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

cyx1231st and others added 14 commits February 28, 2025 09:29
@cyx1231st @myoungwon
crimson/os/seastore: drop unnecessary do_omap_get_values()
2b2b17a 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
Signed-off-by: Myoungwon Oh <ohmyoungwon@gmail.com>
@cyx1231st @myoungwon
crimson/os/seastore: cleanup _omap_set_kvs()
3133388 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
@cyx1231st @myoungwon
crimson/os/seastore: drop unnecessary _omap_set_kvs()
9b2e474 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
@myoungwon
crimson/os/seastore: introduce NONE to omap_type_t
470b6fa 
Signed-off-by: Myoungwon Oh <ohmyoungwon@gmail.com>
@cyx1231st @myoungwon
crimson/os/seastore: various cleanups and indention adjustments
d41a676 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
Signed-off-by: Myoungwon Oh <ohmyoungwon@gmail.com>
@cyx1231st @myoungwon
crimson/os/seastore: drop unnecessary _xattr_clear()
43f9eeb 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
@cyx1231st @myoungwon
crimson/os/seastore: introduce omaptree_clear()
2c03e77 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
Signed-off-by: Myoungwon Oh <ohmyoungwon@gmail.com>
@cyx1231st @myoungwon
crimson/os/seastore: introduce omaptree_update_root()
323ef51 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
@cyx1231st @myoungwon
crimson/os/seastore: cleanups around omap_root_t
7b96626 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
Signed-off-by: Myoungwon Oh <ohmyoungwon@gmail.com>
@cyx1231st @myoungwon
crimson/os/seastore: introduce get_omap_root()
8a7bad0 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
Signed-off-by: Myoungwon Oh <ohmyoungwon@gmail.com>
@cyx1231st @myoungwon
crimson/os/seastore: unify Transaction& parameter for omap
6008796 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
Signed-off-by: Myoungwon Oh <ohmyoungwon@gmail.com>
@cyx1231st @myoungwon
crimson/os/seastore: pass onode as reference whenever possible
e3755b0 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
@cyx1231st @myoungwon
crimson/os/seastore: unify omaptree interfaces
1cb13e8 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
Signed-off-by: Myoungwon Oh <ohmyoungwon@gmail.com>
@cyx1231st @myoungwon
crimson/os/seastore: improve omaptree logs
d137978 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
@myoungwon myoungwon requested a review from a team as a code owner March 2, 2025 22:55
@myoungwon
crimson/os/seastore: group omaptree-related implementations
f8c794f 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
Signed-off-by: Myoungwon Oh <ohmyoungwon@gmail.com>
@myoungwon myoungwon force-pushed the seastore-omap-log-cleanup branch from fd5b913 to f8c794f Compare March 3, 2025 02:09
@cyx1231st cyx1231st added this to Crimson Mar 4, 2025
@cyx1231st cyx1231st moved this to In Progress in Crimson Mar 4, 2025
@cyx1231st
Copy link
Member

cyx1231st commented Mar 6, 2025

jenkins retest this please

1 similar comment
@cyx1231st
Copy link
Member

cyx1231st commented Mar 12, 2025

jenkins retest this please

@myoungwon myoungwon changed the title [WIP] crimson/os/seastore: cleanups related to managing omap and log tree crimson/os/seastore: cleanups related to managing omap and log tree Mar 12, 2025
@myoungwon
Copy link
Member Author

myoungwon commented Mar 12, 2025 •
edited
Loading

@cyx1231st I performed a rebase based on the recent changes. Most of the commit need to be changed due to the recent change , so I remade some of them. Can you take a look?

@cyx1231st
Copy link
Member

cyx1231st commented Mar 12, 2025

I'll review it soon. I'm still going through another PR, nearly done.

@cyx1231st cyx1231st self-requested a review March 12, 2025 02:19
@cyx1231st
Copy link
Member

cyx1231st commented Mar 12, 2025

Looks transaction_manager_test/tm_multi_tier_device_test_t.evict/0 is somehow stalled.

@cyx1231st
Copy link
Member

cyx1231st commented Mar 12, 2025

jenkins retest this please

1 similar comment
@cyx1231st
Copy link
Member

cyx1231st commented Mar 12, 2025

jenkins retest this please

cyx1231st
cyx1231st reviewed Mar 12, 2025
View reviewed changes
@cyx1231st
Copy link
Member

cyx1231st commented Mar 12, 2025 •
edited
Loading

(unrelated to this PR, for #59213)

Seems to me SeaStore::Shard::_omap_clear() should clear omap_type_t::LOG as well. What do you think? @myoungwon

@myoungwon
Copy link
Member Author

myoungwon commented Mar 12, 2025

(unrelated to this PR, for #59213)

Seems to me SeaStore::Shard::_omap_clear() should clear omap_type_t::LOG as well. What do you think? @myoungwon

Right. I'll add a commit to address it.

myoungwon and others added 5 commits March 13, 2025 02:11
@myoungwon
crimson/os/seastore: clear an omap root for log during _omap_clear
0140e6d 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
Signed-off-by: Myoungwon Oh <ohmyoungwon@gmail.com>
@cyx1231st
crimson/os/seastore: reuse get_omap_root()
5b97e49 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
@cyx1231st
crimson/os/seastore: introduce and use omaptree_initialize()
c66e46f 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
@cyx1231st
crimson/os/seastore: prevent omaptree_rm_key() to be called concurren…
ef2b82a 
…tly in the same transaction

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
@cyx1231st
crimson/os/seastore: introduce rename_omap_root() with src and dest o…
5bcccab 
…nodes

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
@cyx1231st
Copy link
Member

cyx1231st commented Mar 13, 2025

@myoungwon I appended 4 commits for further cleanups and fixes, please take a look.

@zhscn I grouped the usages of onode.get_metadata_hint() in seastore.cc. I think this can be cleaner for the hint redesign.

@myoungwon
Copy link
Member Author

myoungwon commented Mar 13, 2025

The changes look good to me.

@myoungwon
Copy link
Member Author

myoungwon commented Mar 13, 2025

jenkins test api

1 similar comment
@cyx1231st
Copy link
Member

cyx1231st commented Mar 14, 2025

jenkins test api

@cyx1231st
Copy link
Member

cyx1231st commented Mar 14, 2025

jenkins test make check

@cyx1231st
Copy link
Member

cyx1231st commented Mar 14, 2025

jenkins test api

@cyx1231st cyx1231st moved this from In Progress to Needs QA in Crimson Mar 14, 2025
@cyx1231st
Copy link
Member

cyx1231st commented Mar 14, 2025 •
edited
Loading

https://shaman.ceph.com/builds/ceph/ci-yingxin-seastore-omap-refactor/

https://pulpito.ceph.com/yingxin-2025-03-14_02:20:56-crimson-rados-ci-yingxin-seastore-omap-refactor-distro-default-smithi/
10/10 tests passing

https://pulpito.ceph.com/yingxin-2025-03-14_03:07:39-crimson-rados-ci-yingxin-seastore-omap-refactor-distro-default-smithi/

@cyx1231st
Copy link
Member

cyx1231st commented Mar 14, 2025

jenkins test make check

2 similar comments
@myoungwon
Copy link
Member Author

myoungwon commented Mar 15, 2025

jenkins test make check

@cyx1231st
Copy link
Member

cyx1231st commented Mar 17, 2025

jenkins test make check

@cyx1231st cyx1231st merged commit 352d1a4 into ceph:main Mar 17, 2025
11 checks passed
@cyx1231st cyx1231st moved this from Needs QA to Merged in Crimson Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cyx1231st cyx1231st cyx1231st approved these changes

Assignees

No one assigned

Labels

cleanup crimson seastore TESTED

Projects

Crimson
Status: Merged (Pre Tentacle Freeze)

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@myoungwon @cyx1231st