Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/bluestore: don't forget recalc_allocated for log_file when decode(… #20662

Closed

Conversation

majianpeng
Copy link
Member

…super)

This cause the following bugs:
0> 2018-03-01 01:12:07.045 7fe8b7e71e00 -1 /root/ceph/src/os/bluestore/BlueFS.cc: In function 'int BlueFS::_flush_range(BlueFS::FileWriter*, uint64_t, uint64_t)' thread 7fe8b7e71e00 time 2018-03-01 01:12:07.042115
/root/ceph/src/os/bluestore/BlueFS.cc: 1661: FAILED assert(h->file->fnode.ino != 1)

ceph version 13.0.1-2318-ga14754f1eb (a14754f) mimic (dev)
1: (ceph::__ceph_assert_fail(char const*, char const*, int, char const*)+0x105) [0x7fe8af2da1e5]
2: (BlueFS::_flush_range(BlueFS::FileWriter*, unsigned long, unsigned long)+0xf13) [0x55ba9a71c503]
3: (BlueFS::_flush(BlueFS::FileWriter*, bool)+0x146) [0x55ba9a71c726]
4: (BlueFS::_flush_and_sync_log(std::unique_lockstd::mutex&, unsigned long, unsigned long)+0x4a4) [0x55ba9a71cfb4]
5: (BlueFS::_fsync(BlueFS::FileWriter*, std::unique_lockstd::mutex&)+0xa1) [0x55ba9a71d7f1]
6: (BlueRocksWritableFile::Sync()+0x63) [0x55ba9a730113]
7: (rocksdb::WritableFileWriter::SyncInternal(bool)+0x256) [0x55ba9a8c5166]
8: (rocksdb::WritableFileWriter::Sync(bool)+0xd8) [0x55ba9a8c5e38]
9: (rocksdb::BuildTable(std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&, rocksdb::Env*, rocksdb::ImmutableCFOptions const&, rocksdb::MutableCFOptions const&, rocksdb::EnvOptions const&, rocksdb::TableCache*, rocksdb::InternalIterator*, std::unique_ptr<rocksdb::InternalIterator, std::default_deleterocksdb::InternalIterator >, rocksdb::FileMetaData*, rocksdb::InternalKeyComparator const&, std::vector<std::unique_ptr<rocksdb::IntTblPropCollectorFactory, std::default_deleterocksdb::IntTblPropCollectorFactory >, std::allocator<std::unique_ptr<rocksdb::IntTblPropCollectorFactory, std::default_deleterocksdb::IntTblPropCollectorFactory > > > const*, unsigned int, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&, std::vector<unsigned long, std::allocator >, unsigned long, rocksdb::SnapshotChecker*, rocksdb::CompressionType, rocksdb::CompressionOptions const&, bool, rocksdb::InternalStats*, rocksdb::TableFileCreationReason, rocksdb::EventLogger*, int, rocksdb::Env::IOPriority, rocksdb::TableProperties*, int, unsigned long)+0x1c67) [0x55ba9a8e4037]
10: (rocksdb::DBImpl::WriteLevel0TableForRecovery(int, rocksdb::ColumnFamilyData*, rocksdb::MemTable*, rocksdb::VersionEdit*)+0xc13) [0x55ba9a7c4663]
11: (rocksdb::DBImpl::RecoverLogFiles(std::vector<unsigned long, std::allocator > const&, unsigned long*, bool)+0x1552) [0x55ba9a7c6602]
12: (rocksdb::DBImpl::Recover(std::vector<rocksdb::ColumnFamilyDescriptor, std::allocatorrocksdb::ColumnFamilyDescriptor > const&, bool, bool, bool)+0x881) [0x55ba9a7c7b71]
13: (rocksdb::DB::Open(rocksdb::DBOptions const&, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&, std::vector<rocksdb::ColumnFamilyDescriptor, std::allocatorrocksdb::ColumnFamilyDescriptor > const&, std::vector<rocksdb::ColumnFamilyHandle*, std::allocatorrocksdb::ColumnFamilyHandle* >, rocksdb::DB**)+0x649) [0x55ba9a7c85e9]
14: (RocksDBStore::do_open(std::ostream&, bool, std::vector<KeyValueDB::ColumnFamily, std::allocatorKeyValueDB::ColumnFamily > const
)+0xfbc) [0x55ba9a6c927c]
15: (BlueStore::_open_db(bool, bool)+0xcce) [0x55ba9a666c2e]
16: (BlueStore::_mount(bool, bool)+0x1ea) [0x55ba9a68dcaa]
17: (OSD::init()+0x296) [0x55ba9a2e7d26]
18: (main()+0x2862) [0x55ba9a1e8bb2]
19: (__libc_start_main()+0xf1) [0x7fe8acf021c1]
20: (_start()+0x2a) [0x55ba9a2b44ea]

Signed-off-by: Jianpeng Ma jianpeng.ma@intel.com

@majianpeng
Copy link
Member Author

@liewegas . I think this is a serious bug. please review.

…super)

This cause the following bugs:
     0> 2018-03-01 01:12:07.045 7fe8b7e71e00 -1 /root/ceph/src/os/bluestore/BlueFS.cc: In function 'int BlueFS::_flush_range(BlueFS::FileWriter*, uint64_t, uint64_t)' thread 7fe8b7e71e00 time 2018-03-01 01:12:07.042115
/root/ceph/src/os/bluestore/BlueFS.cc: 1661: FAILED assert(h->file->fnode.ino != 1)

 ceph version 13.0.1-2318-ga14754f1eb (a14754f) mimic (dev)
 1: (ceph::__ceph_assert_fail(char const*, char const*, int, char const*)+0x105) [0x7fe8af2da1e5]
 2: (BlueFS::_flush_range(BlueFS::FileWriter*, unsigned long, unsigned long)+0xf13) [0x55ba9a71c503]
 3: (BlueFS::_flush(BlueFS::FileWriter*, bool)+0x146) [0x55ba9a71c726]
 4: (BlueFS::_flush_and_sync_log(std::unique_lock<std::mutex>&, unsigned long, unsigned long)+0x4a4) [0x55ba9a71cfb4]
 5: (BlueFS::_fsync(BlueFS::FileWriter*, std::unique_lock<std::mutex>&)+0xa1) [0x55ba9a71d7f1]
 6: (BlueRocksWritableFile::Sync()+0x63) [0x55ba9a730113]
 7: (rocksdb::WritableFileWriter::SyncInternal(bool)+0x256) [0x55ba9a8c5166]
 8: (rocksdb::WritableFileWriter::Sync(bool)+0xd8) [0x55ba9a8c5e38]
 9: (rocksdb::BuildTable(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, rocksdb::Env*, rocksdb::ImmutableCFOptions const&, rocksdb::MutableCFOptions const&, rocksdb::EnvOptions const&, rocksdb::TableCache*, rocksdb::InternalIterator*, std::unique_ptr<rocksdb::InternalIterator, std::default_delete<rocksdb::InternalIterator> >, rocksdb::FileMetaData*, rocksdb::InternalKeyComparator const&, std::vector<std::unique_ptr<rocksdb::IntTblPropCollectorFactory, std::default_delete<rocksdb::IntTblPropCollectorFactory> >, std::allocator<std::unique_ptr<rocksdb::IntTblPropCollectorFactory, std::default_delete<rocksdb::IntTblPropCollectorFactory> > > > const*, unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<unsigned long, std::allocator<unsigned long> >, unsigned long, rocksdb::SnapshotChecker*, rocksdb::CompressionType, rocksdb::CompressionOptions const&, bool, rocksdb::InternalStats*, rocksdb::TableFileCreationReason, rocksdb::EventLogger*, int, rocksdb::Env::IOPriority, rocksdb::TableProperties*, int, unsigned long)+0x1c67) [0x55ba9a8e4037]
 10: (rocksdb::DBImpl::WriteLevel0TableForRecovery(int, rocksdb::ColumnFamilyData*, rocksdb::MemTable*, rocksdb::VersionEdit*)+0xc13) [0x55ba9a7c4663]
 11: (rocksdb::DBImpl::RecoverLogFiles(std::vector<unsigned long, std::allocator<unsigned long> > const&, unsigned long*, bool)+0x1552) [0x55ba9a7c6602]
 12: (rocksdb::DBImpl::Recover(std::vector<rocksdb::ColumnFamilyDescriptor, std::allocator<rocksdb::ColumnFamilyDescriptor> > const&, bool, bool, bool)+0x881) [0x55ba9a7c7b71]
 13: (rocksdb::DB::Open(rocksdb::DBOptions const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<rocksdb::ColumnFamilyDescriptor, std::allocator<rocksdb::ColumnFamilyDescriptor> > const&, std::vector<rocksdb::ColumnFamilyHandle*, std::allocator<rocksdb::ColumnFamilyHandle*> >*, rocksdb::DB**)+0x649) [0x55ba9a7c85e9]
 14: (RocksDBStore::do_open(std::ostream&, bool, std::vector<KeyValueDB::ColumnFamily, std::allocator<KeyValueDB::ColumnFamily> > const*)+0xfbc) [0x55ba9a6c927c]
 15: (BlueStore::_open_db(bool, bool)+0xcce) [0x55ba9a666c2e]
 16: (BlueStore::_mount(bool, bool)+0x1ea) [0x55ba9a68dcaa]
 17: (OSD::init()+0x296) [0x55ba9a2e7d26]
 18: (main()+0x2862) [0x55ba9a1e8bb2]
 19: (__libc_start_main()+0xf1) [0x7fe8acf021c1]
 20: (_start()+0x2a) [0x55ba9a2b44ea]

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
@majianpeng
Copy link
Member Author

@liewegas . Ping.

@@ -51,6 +51,7 @@ void bluefs_super_t::decode(bufferlist::iterator& p)
decode(block_size, p);
decode(log_fnode, p);
DECODE_FINISH(p);
log_fnode.recalc_allocated();
Copy link
Contributor

@tchaikov tchaikov Mar 2, 2018

Choose a reason for hiding this comment

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

i'd suggest update the DENC in bluefs_fnode_t instead, so it will update its allocated at end of decode(). and also drop the fnode.recalc_allocated() call in BlueFS::_replay() accordingly.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@majianpeng
Copy link
Member Author

@tchaikov . By your suggestion move recalc_allocated in DENC. But met the following error:
/mnt/ceph/src/os/bluestore/bluefs_types.h: In instantiation of ‘std::enable_if_t<(is_same_v<T, bluefs_fnode_t> || is_same_v<T, const bluefs_fnode_t>)> _denc_friend(T&, P&) [with T = const bluefs_fnode_t; P = long unsigned int; std::enable_if_t<(is_same_v<T, bluefs_fnode_t> || is_same_v<T, const bluefs_fnode_t>)> = void]’:
/mnt/ceph/src/os/bluestore/bluefs_types.h:57:3: required from here
/mnt/ceph/src/os/bluestore/bluefs_types.h:67:17: error: assignment of member ‘bluefs_fnode_t::allocated’ in read-only object
v.allocated = allocated;
~~~~~~~~~~~~^~~~~~~~~~~

And i didn't find any example for this. Could you give me a example? Thanks!

@tchaikov
Copy link
Contributor

tchaikov commented Mar 5, 2018

@majianpeng ahh, i underestimated the effort to have the proposed fix. but i pulled together a revised one at #20701. could you take a look at it ? if it looks sane to you, i'd suggest close your fix and use it instead as it's more error-prone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants