Bug #42516
closedmds: some mutations have initiated (TrackedOp) set to 0
0%
Description
From Brad:
I was looking for tracker ops that had been created with 'initiated'
set to zero and came across this code.src/mds/Mutation.h:
158 // keep our default values synced with MDRequestParam's
159 MutationImpl() : TrackedOp(nullptr, utime_t()) {}This default constructor is used quite a bit and results in TrackedOps
with 'initiated' set to zero.src/mds/Locker.cc
2447: MutationRef mut(new MutationImpl());
3157: MutationRef mut = new MutationImpl();
3449: MutationRef mut(new MutationImpl());
4479: MutationRef mut(new MutationImpl());src/mds/Migrator.cc
1334: it->second.mut = new MutationImpl();
2583: it->second.mut = new MutationImpl();src/mds/CInode.cc
2258: MutationRef mut(new MutationImpl());src/mds/MDCache.cc
528: MutationRef mut(new MutationImpl());
6446: MutationRef mut(new MutationImpl());I'm not suggesting this is the source of the issue since I would have
no idea how the creation of a MutationImpl object would translate into
an MOSDECSubOpRead/Reply on the OSD nor how this would possibly
affect rgw or rbd tests but I thought I'd see what you thought about
this code.
This may not be a problem as the value maybe gets set some other way?
Updated by Patrick Donnelly over 3 years ago
- Related to Bug #41834: qa: EC Pool configuration and slow op warnings for OSDs caused by recent master changes added
Updated by Ramana Raja almost 3 years ago
- Status changed from New to In Progress
Updated by Ramana Raja almost 3 years ago
I checked Migrator.cc for creation of MutationImpl object and setting of its TrackedOp initiated_at attribute mentioned in the Description.
src/mds/Migrator.cc
1334: it->second.mut = new MutationImpl();
2583: it->second.mut = new MutationImpl();
The above two happens at Migrator::export_frozen and Migrator::handle_export_prep respectively. In both the methods, the MutationImpl/TrackedOp "initiated_at" of a CDir's export_state_t is not set anywhere else later in the code. I'm guessing the TrackedOp "initiated_at" is not used/useful in Migrator related code.
In src/mds/MDCache.cc,
MDCache.cc:534: MutationRef mut(new MutationImpl());
The above code is in MDCache::_create_system_file
MDCache.cc:6489: MutationRef mut(new MutationImpl());
The above code is in MDCache::truncate_inode_finish
MDCache.cc:12194: MutationRef mut(new MutationImpl());
The above code is in MDCache:: MDCache::rollback_uncommitted_fragments
In src/mds/CInode.cc,
CInode.cc:2356: MutationRef mut(new MutationImpl());
The above code is in CInode::finish_scatter_update
In src/mds/StrayManager.cc,
StrayManager.cc:183: MutationRef mut(new MutationImpl());
StrayManager.cc:221: MutationRef mut(new MutationImpl());
The above code is in StrayManager::_purge_stray_purged
In all the above occurrences, the "initated_at" is not set anywhere else in the respective methods or in their calling methods.
Looking at the MDS code base, MDRequestRef TrackedOps object "initiated_at" is set at MDCache::request_start, MDCache::request_start_internal, and MDCache::request_start_peer. e.g., MDCache::request_start gets called in Server::handle_client_request.
Updated by Patrick Donnelly almost 3 years ago
Ramana Raja wrote:
I checked Migrator.cc for creation of MutationImpl object and setting of its TrackedOp initiated_at attribute mentioned in the Description.
src/mds/Migrator.cc
1334: it->second.mut = new MutationImpl();
2583: it->second.mut = new MutationImpl();The above two happens at Migrator::export_frozen and Migrator::handle_export_prep respectively. In both the methods, the MutationImpl/TrackedOp "initiated_at" of a CDir's export_state_t is not set anywhere else later in the code. I'm guessing the TrackedOp "initiated_at" is not used/useful in Migrator related code.
In src/mds/MDCache.cc,
MDCache.cc:534: MutationRef mut(new MutationImpl());
The above code is in MDCache::_create_system_file
MDCache.cc:6489: MutationRef mut(new MutationImpl());
The above code is in MDCache::truncate_inode_finish
MDCache.cc:12194: MutationRef mut(new MutationImpl());
The above code is in MDCache:: MDCache::rollback_uncommitted_fragments
In src/mds/CInode.cc,
CInode.cc:2356: MutationRef mut(new MutationImpl());
The above code is in CInode::finish_scatter_update
In src/mds/StrayManager.cc,
StrayManager.cc:183: MutationRef mut(new MutationImpl());
StrayManager.cc:221: MutationRef mut(new MutationImpl());The above code is in StrayManager::_purge_stray_purged
In all the above occurrences, the "initated_at" is not set anywhere else in the respective methods or in their calling methods.
Looking at the MDS code base, MDRequestRef TrackedOps object "initiated_at" is set at MDCache::request_start, MDCache::request_start_internal, and MDCache::request_start_peer. e.g., MDCache::request_start gets called in Server::handle_client_request.
I don't see a way to updated initiated_at post-creation for TrackedOp. I think the right change here is to initialize with the current time rather than utime_t(). What do you think?
Updated by Ramana Raja almost 3 years ago
- Status changed from In Progress to Fix Under Review
- Pull request ID set to 41371
Updated by Patrick Donnelly almost 3 years ago
- Status changed from Fix Under Review to Resolved
- Target version set to v17.0.0