MDSMonitor: silence unable to load metadata #25693
Conversation
|
@xiexingguo please take a review, thanks |
src/mon/MDSMonitor.cc
Outdated
| @@ -1641,7 +1641,9 @@ int MDSMonitor::load_metadata(map<mds_gid_t, Metadata>& m) | |||
| bufferlist bl; | |||
| int r = mon->store->get(MDS_METADATA_PREFIX, "last_metadata", bl); | |||
| if (r) { | |||
| dout(1) << "Unable to load 'last_metadata'" << dendl; | |||
| if (r != -ENOENT) { | |||
There was a problem hiding this comment.
this doesn't sound right. The underlying store is only propagating one error via the return value, and that is ENOENT. What you are doing is silently creating a dead branch that will always suppress the error message.
I'm not sure what problem you are trying to address, but this doesn't seem the right way to do it.
There was a problem hiding this comment.
thanks for your review @jecluis
logging this message when quering cyclicly(ceph mds versions) seems useless and wastes space. we'd better silence it at least in normal use case.
i can think of two ways to optimize this as only ENOENT exists, the first one is just return,(
Line 1045 in 0ccdc79
which one do you prefer? or any other ideas ?
There was a problem hiding this comment.
@tchaikov as it seems jecluis busy with other things., please take a review if you have time.
There was a problem hiding this comment.
will do the day after tomorrow.
There was a problem hiding this comment.
I don't think we can, in good conscious, suppress these messages. The message is there to make it clear that the key does not exist, and I think it is very worthwhile for those cases where it is supposed to be there.
I think the best solution is to check whether there are mds in the mdsmap, and only call count_metadata() if so, for all the callers of count_metadata(). Or, maybe simpler, do that in count_metadata() itself. I think this will require adding a getter method in FSMap though (something like has_daemons()), because I don't think we currently expose this easily without calling something like FSMap::get_mds_info() and checking if it's empty.
There was a problem hiding this comment.
!_!, the best solution you proposed is not ready now, and i agree with the opinion that these messages are worthwhile.
could we just increase the dout level, say from dout(1) -> dout(5) ?
because on one hand, these message are worthwhile but not urgent, on the other hand, these messages will be slienced at usual as we want.
anyway, thanks for your detailed explaination and guidance.
There was a problem hiding this comment.
@shun-s sure, lets go with dout(5) then.
There was a problem hiding this comment.
@jecluis repush done.
batrick
changed the title
silence unable to load metadata Signed-off-by: Song Shun <song.shun3@zte.com.cn>
shun-s
force-pushed
the
silence-mon-load-metadata
branch
from
05fa1fb
to
c4c73e6
Compare
silence unable to load metadata
Signed-off-by: Song Shun song.shun3@zte.com.cn