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

Fix Ceph build broken in AArm64 architecture #10

Closed
wants to merge 2 commits into from

Conversation

tone-zhang
Copy link

In ARM the "char" type is treated as "unsigned char", the value
range is from 0 to 255. In the two files options.h and
perf_level.h, the value of the enum type is assigned to "-1".
It is out of the scope. Ceph build will be broken because of the
issue.
The issue has been fix in facebook/master, rebase the commit
(facebook/rocksdb@9142f29) to ceph/rocksdb.
Also rebase the commit (facebook/rocksdb@2ea2d92) to
ceph/rocksdb for GCC5.4 compitalbe.

fceller and others added 2 commits July 13, 2016 08:42
GCC 5.4 will complain (see also options_parser.cc):

    /home/abuild/rpmbuild/BUILD/arangodb-3.0.0r1/3rdParty/rocksdb/rocksdb/util/options_builder.cc: In function 'rocksdb::CompactionStyle rocksdb::{anonymous}::PickCompactionStyle(size_t, int, int, uint64_t)':
    /home/abuild/rpmbuild/BUILD/arangodb-3.0.0r1/3rdParty/rocksdb/rocksdb/util/options_builder.cc:29:7: error: 'log' is not a member of 'std'
           std::log(target_db_size / write_buffer_size) / std::log(kBytesForLevelMultiplier)));
           ^
    /home/abuild/rpmbuild/BUILD/arangodb-3.0.0r1/3rdParty/rocksdb/rocksdb/util/options_builder.cc:29:7: note: suggested alternative:
    In file included from /usr/include/features.h:365:0,
                     from /usr/include/math.h:26,
                     from /home/abuild/rpmbuild/BUILD/arangodb-3.0.0r1/3rdParty/rocksdb/rocksdb/util/options_builder.cc:6:
    /usr/include/bits/mathcalls.h:109:1: note:   'log'
     __MATHCALL_VEC (log,, (_Mdouble_ __x));
Summary: char is not signed in some platforms. Having negative values confuse those compilers.

Test Plan: Run all existing tests.

Reviewers: andrewkr, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: leveldb, andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D59619
@tchaikov
Copy link

tchaikov commented Jul 15, 2016

@tone-zhang cherry-picking from the upstream is less optimal, i manually forwarded the master branch to pick up the latest change of the upstream. so maybe we can just point the submodule in ceph to the HEAD of ceph/rocksdb. what do you think?

@tone-zhang
Copy link
Author

@tchaikov Thanks a lot! I agree with you. In fact, there are over 40 commits between the upstream and ceph/rocksdb. I'm just a little worry that it will introduce regression or not. Do you mean we need to "rebase" all the latest changes of the upstream to ceph/rocksdb?

@tchaikov
Copy link

@tone-zhang, i "fast-forwarded" the master branch of ceph/rocksdb to the master branch's HEAD of facebook/rocksb. so no need to "rebase" or "cherry-pick" or "merge" anymore. we just need to update the submodule in ceph/ceph now.

@dmick
Copy link
Member

dmick commented Jul 22, 2016

The aarch64 build is still broken. What's the plan here?

@dmick
Copy link
Member

dmick commented Jul 22, 2016

reading more carefully: this PR should be closed, correct?

@tchaikov
Copy link

@dmick yeah, i am closing it.

@tchaikov tchaikov closed this Jul 24, 2016
tchaikov pushed a commit to tchaikov/rocksdb that referenced this pull request Sep 8, 2016
Summary:
We've got a crash with this stack trace:

  Program terminated with signal SIGTRAP, Trace/breakpoint trap.

  #0  0x00007fc85f2f4009 in raise () from /usr/local/fbcode/gcc-4.9-glibc-2.20-fb/lib/libpthread.so.0
  ceph#1  0x00000000005c8f61 in facebook::logdevice::handle_sigsegv(int) () at logdevice/server/sigsegv.cpp:159
  ceph#2  0x00007fc85f2f4150 in <signal handler called> () at /usr/local/fbcode/gcc-4.9-glibc-2.20-fb/lib/libpthread.so.0
  ceph#3  0x00000000031ed80c in rocksdb::NewReadaheadRandomAccessFile() at util/file_reader_writer.cc:383
  ceph#4  0x00000000031ed80c in rocksdb::NewReadaheadRandomAccessFile() at util/file_reader_writer.cc:472
  ceph#5  0x00000000031558e7 in rocksdb::TableCache::GetTableReader() at db/table_cache.cc:99
  ceph#6  0x0000000003156329 in rocksdb::TableCache::NewIterator() at db/table_cache.cc:198
  ceph#7  0x0000000003166568 in rocksdb::VersionSet::MakeInputIterator() at db/version_set.cc:3345
  ceph#8  0x000000000324a94f in rocksdb::CompactionJob::ProcessKeyValueCompaction(rocksdb::CompactionJob::SubcompactionState*) () at db/compaction_job.cc:650
  ceph#9  0x000000000324c2f6 in rocksdb::CompactionJob::Run() () at db/compaction_job.cc:530
  ceph#10 0x00000000030f5ae5 in rocksdb::DBImpl::BackgroundCompaction() at db/db_impl.cc:3269
  ceph#11 0x0000000003108d36 in rocksdb::DBImpl::BackgroundCallCompaction(void*) () at db/db_impl.cc:2970
  ceph#12 0x00000000029a2a9a in facebook::logdevice::RocksDBEnv::callback(void*) () at logdevice/server/locallogstore/RocksDBEnv.cpp:26
  ceph#13 0x00000000029a2a9a in facebook::logdevice::RocksDBEnv::callback(void*) () at logdevice/server/locallogstore/RocksDBEnv.cpp:30
  ceph#14 0x00000000031e7521 in rocksdb::ThreadPool::BGThread() at util/threadpool.cc:230
  ceph#15 0x00000000031e7663 in rocksdb::BGThreadWrapper(void*) () at util/threadpool.cc:254
  ceph#16 0x00007fc85f2ea7f1 in start_thread () at /usr/local/fbcode/gcc-4.9-glibc-2.20-fb/lib/libpthread.so.0
  ceph#17 0x00007fc85e8fb46d in clone () at /usr/local/fbcode/gcc-4.9-glibc-2.20-fb/lib/libc.so.6

From looking at the code, probably what happened is this:
 - `TableCache::GetTableReader()` called `Env::NewRandomAccessFile()`, which dispatched to a `PosixEnv::NewRandomAccessFile()`, where probably an `open()` call failed, so the `NewRandomAccessFile()` left a nullptr in the resulting file,
 - `TableCache::GetTableReader()` called `NewReadaheadRandomAccessFile()` with that `nullptr` file,
 - it tried to call file's method and crashed.

This diff is a trivial fix to this crash.

Test Plan: `make -j check`

Reviewers: sdong, andrewkr, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D62451
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants