Commit Graph

123 Commits

Author SHA1 Message Date
Chris Mumford
4cb80b7ddc Merge pull request #386 from ivanabc:master
PiperOrigin-RevId: 250702492
2019-05-30 09:56:34 -07:00
Victor Costan
72a38ff7f2 Replace "> >" with ">>"
PiperOrigin-RevId: 250383036
2019-05-30 09:55:43 -07:00
Victor Costan
863f185970 unsigned char -> uint8_t
PiperOrigin-RevId: 250309603
2019-05-28 15:44:32 -07:00
ivan
63d5315e1c
Merge branch 'master' into master 2019-05-23 14:02:04 +08:00
Chris Mumford
c00e177f36 Guard DBImpl::versions_ by mutex_.
mutex_ was already acquired before accessing DBImpl::versions_ in all
but one place: DBImpl::GetApproximateSizes. This change requires mutex_
to be held before accessing versions_.

PiperOrigin-RevId: 248390814
2019-05-16 12:07:21 -07:00
Chris Mumford
1d0b101165 Converted two for-loops to while-loops.
Converted `for (;<condition>;)` to `while (<condition>)`.

PiperOrigin-RevId: 247950510
2019-05-13 13:51:11 -07:00
Chris Mumford
28e6d238be Switch to using C++ 11 override specifier.
PiperOrigin-RevId: 247491163
2019-05-09 14:11:06 -07:00
Chris Mumford
85cd40d108 Added unit test for InternalKey::DecodeFrom with empty string.
PiperOrigin-RevId: 247483339
2019-05-09 14:10:55 -07:00
Chris Mumford
1aae5c9f29 Merge pull request #411 from proller:assert1
PiperOrigin-RevId: 247424040
2019-05-09 08:37:49 -07:00
Chris Mumford
b7b86baec9 Using std::ostringstream in key DebugString.
Switching from snprintf to std::ostringstream eliminates
cast warning for (unsigned long long).

PiperOrigin-RevId: 247326681
2019-05-08 17:36:35 -07:00
Victor Costan
4bd052d7e8 Consolidate benchmark code to benchmarks/.
Currently, the benchmark used to assess leveldb changes lives in db/. The codebase also contains two benchmarks against other database engines in doc/bench/. Moving all the benchmarks in one place opens up the way for extracting common code.

PiperOrigin-RevId: 246737541
2019-05-05 12:59:23 -07:00
Victor Costan
506b1722ef Convert missed virtual -> override in db_test.cc.
PiperOrigin-RevId: 246680419
2019-05-04 18:05:53 -07:00
Victor Costan
24424a1ef2 Style cleanup.
1) Convert iterator-based for loops to C++11 foreach loops.
2) Convert "void operator=" to "T& operator=".
3) Switch from copy operators from private to public deleted.
4) Switch from empty ctors / dtors to "= default" where appropriate.

PiperOrigin-RevId: 246679195
2019-05-04 17:42:20 -07:00
Victor Costan
9a56c49ed4 Merge pull request #679 from smartxworks:optimize-readseq
PiperOrigin-RevId: 246668103
2019-05-04 16:53:17 -07:00
Chris Mumford
9bd23c7676 Correct class/structure declaration order.
1. Correct the class/struct declaration order to be IAW
   the Google C++ style guide[1].
2. For non-copyable classes, switched from non-implemented
   private methods to explicitly deleted[2] methods.
3. Minor const and member initialization fixes.

[1] https://google.github.io/styleguide/cppguide.html#Declaration_Order
[2] http://eel.is/c++draft/dcl.fct.def.delete

PiperOrigin-RevId: 246521844
2019-05-03 09:48:57 -07:00
Chris Mumford
297e66afc1 Format all files IAW the Google C++ Style Guide.
Use clang-format to correct formatting to be in agreement with the [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html). Doing this simplifies the process of accepting changes. Also fixed a few warnings flagged by clang-tidy.

PiperOrigin-RevId: 246350737
2019-05-02 19:04:50 -07:00
Kyle Zhang
d3d1c8a0f4 don't check current key in DBIter::Next()
When iter_ is pointing to current key, we can safely move to the next
key to avoid checking current key, which is of course not necessary.

Benchmark shows that 'readseq' has about 8% performance improvement.

Without patch:

>./db_bench --benchmarks=readseq --num=$((4<<20)) --db=/tmp/db --use_existing_db=1
LevelDB:    version 1.21
Date:       Thu Apr 25 09:37:21 2019
CPU:        32 * Intel(R) Xeon(R) CPU E5-2620 v4 @ 2.10GHz
CPUCache:   20480 KB
Keys:       16 bytes each
Values:     100 bytes each (50 bytes after compression)
Entries:    4194304
RawSize:    464.0 MB (estimated)
FileSize:   264.0 MB (estimated)
------------------------------------------------
readseq      :       0.196 micros/op;  565.7 MB/s

With patch:

>./db_bench --benchmarks=readseq --num=$((4<<20)) --db=/tmp/db --use_existing_db=1
LevelDB:    version 1.21
Date:       Thu Apr 25 09:38:20 2019
CPU:        32 * Intel(R) Xeon(R) CPU E5-2620 v4 @ 2.10GHz
CPUCache:   20480 KB
Keys:       16 bytes each
Values:     100 bytes each (50 bytes after compression)
Entries:    4194304
RawSize:    464.0 MB (estimated)
FileSize:   264.0 MB (estimated)
------------------------------------------------
readseq      :       0.181 micros/op;  612.3 MB/s

Signed-off-by: Kyle Zhang <kyle@smartx.com>
2019-04-25 09:54:05 +08:00
Chris Mumford
2f008ac19e Initialize class members to default values in constructors.
There were a few members which were identified to have been left
uninitialized in some constructors. These were very likely to
have been set before being used, otherwise the ASan tests would
have caught them, but still good practice to have them
initialized. This addresses some items reported in issue #668.

PiperOrigin-RevId: 243370145
2019-04-12 18:50:15 -07:00
Chris Mumford
65e86f75ea Fix formatting of recent snapshot compaction fix.
Fix variable names, line lengths, namespace use, and a few other
minor issues to conform to Google C++ style guide.

PiperOrigin-RevId: 243187729
2019-04-12 00:17:03 -07:00
Chris Mumford
7711e76766 Merge pull request #339 from richcole-at-amazon:master
PiperOrigin-RevId: 243156105
2019-04-12 00:16:52 -07:00
Chris Mumford
71ed7c401e Fixed typo in comment in version_set.h.
Flagged by presubmit check.

PiperOrigin-RevId: 243118632
2019-04-12 00:16:41 -07:00
Richard Cole
20fb601aa9 Fix snapshot compaction bug
Closes google/leveldb#320

During compaction it was possible that records from a block b1=(l1,u1)
would be pushed down from level i to level i+1. If there is a block
b2=(l2,u2) at level i with k1 = user_key(u1) = user_key(l2) then
a subsequent search for k1 will yield the record l2 which has a smaller
sequence number than u1 because the sort order for records sorts
increasing by user key but decreaing by sequence number.

This change add a call to a new function AddBoundaryInputs to
SetupOtherInputs. AddBoundaryInputs searches for a block b2 matching the
criteria above and adds it to the set of files to be compacted. Whenever
AddBoundaryInputs is called it is important that the compaction fileset
in level i+1 (known as c->inputs_[1] in the code) be recomputed. Each
call to AddBoundaryInputs is followed by a call to GetOverlappingInputs.

SetupOtherInputs is called on both manual and automated compaction
passes. It is called for both level zero and for levels greater than 0.

The original change posted in https://github.com/google/leveldb/pull/339
has been modified to also include changed made by Chris Mumford<cmumford@google.com>
in 4b72cb14f8

  1. Releasing snapshots during test cleanup to avoid
     memory leak warnings.
  2. Refactored test to use testutil.h to be in line
     with other issue tests and to create the test
     database in the correct temporary location.
  3. Added copyright banner.

  Otherwise, just minor formatting and limiting character
  width to 80 characters.

Additionally the change was rebased on top of current master and
changes previously made to the Makefile were ported to the
CMakeLists.txt.

Testing Done:

  A test program (issue320_test) was constructed that performs mutations
  while snapshots are active. issue320_test fails without this bug fix
  after 64k writes. It passes with this bug fix. It was run with 200M
  writes and passed.

  Unit tests were written for the new function that was added to the
  code. Make test was run and seen to pass.

Signed-off-by: Richard Cole <richcole@amazon.com>
2019-04-01 13:10:09 -07:00
costan
6188a54ce9 leveldb: Add tests for empty keys and values.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=239695281
2019-03-29 11:22:22 -07:00
usurai
6571279d6d fix a typo in the comment of skiplist_test.cc (#664) 2019-03-21 07:58:29 -07:00
cmumford
ea49b27d06 Switch corruption_test to use InMemEnv.
This change switches corruption_test, which previously used direct file
I/O to corrupt table files for open databases, to use InMemEnv. Using an
Env eliminates some platform dependencies thus simplifying the tests.

Also removed EnvWindowsTestHelper::RelaxFilePermissions().  This was
only added because the Windows Env opens files for exclusive access.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=239305329
2019-03-20 13:57:03 -07:00
costan
201f77d137 Inline defaults in options.
This CL moves default values for
leveldb::{Options,ReadOptions,WriteOptions} from constructors to member
declarations, and removes now-redundant comments stating the defaults.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=239271242
2019-03-20 13:56:22 -07:00
costan
7d8e41e49b leveldb: Replace AtomicPointer with std::atomic.
This CL removes AtomicPointer from leveldb's port interface. Its usage is replaced with std::atomic<> from the C++11 standard library.

AtomicPointer was used to wrap flags, numbers, and pointers, so its instances are replaced with std::atomic<bool>, std::atomic<int>, std::atomic<size_t> and std::atomic<Node*>.

This CL does not revise the memory ordering. AtomicPointer's methods are replaced mechanically with their std::atomic equivalents, even when the underlying usage is incorrect. (Example: DBImpl::has_imm_ is written using release stores, even though it is always read using relaxed ordering.) Revising the memory ordering is left for future CLs.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=237865146
2019-03-11 13:41:25 -07:00
Dimitris Apostolou
a20508dc6a Fix typo (#565) 2019-03-11 10:36:11 -07:00
cmumford
c69d33b0ec Added native support for Windows.
This change adds a native Windows port (port_windows.h) and a
Windows Env (WindowsEnv).

Note1: "small" is defined when including <Windows.h> so some
parameters were renamed to avoid conflict.

Note2: leveldb::Env defines the method: "DeleteFile" which is
also a constant defined when including <Windows.h>. The solution
was to ensure this macro is defined in env.h which forces
the function, when compiled, to be either DeleteFileA or
DeleteFileW when building for MBCS or UNICODE respectively.

This resolves #519 on GitHub.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=236364778
2019-03-01 18:00:35 -08:00
costan
fe4494804f leveldb: Make WriteBatch::ApproximateSize() const.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=229395810
2019-01-15 18:43:13 +00:00
costan
89af27bde5 Remove ssize_t from code that is not POSIX-specific.
ssize_t is not standard C++. It is a POSIX extension. Therefore, it does
not belong in generic code.

This change tweaks the logic in DBIter to remove the need for signed
integers, so ssize_t can be replaced with size_t. The impacted method
and private member are renamed to better express their purpose.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=211471606
2018-09-04 10:37:22 -07:00
costan
16a2b8bb3a Expose WriteBatch::Append in the C API.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=209345072
2018-08-19 19:54:34 -07:00
costan
f7b0e1d901 Expose WriteBatch::Append().
WriteBatchInternal has a method for efficiently concatenating two
WriteBatches. This commit exposes the method to the public API.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=208724311
2018-08-14 15:30:29 -07:00
costan
1868398150 Clean up SnapshotImpl.
* Omit SnapshotImpl::list_ when assert() isn't on
* Make SnapshotImpl::number_ const and set it in the constructor
* Make SnapshotImpl::number_ private and access it via a getter
* Rename SnapshotImpl::number_ to SnapshotImpl::sequence_number_
* Rename SnapshotList::list_ to SnapshotList::head_
* Wrap casting from Snapshot* to SnapshotImpl* in ToSnapshotImpl()

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=194852828
2018-04-30 16:01:39 -07:00
MarcoFalke
14cce848e7 Fix sign mismatch warnings in GCC.
This was contributed in https://github.com/google/leveldb/pull/492

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=193080913
2018-04-16 18:13:09 -07:00
costan
09217fd067 Replace NULL with nullptr in C++ files.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=192365747
2018-04-10 16:26:43 -07:00
costan
0db30413a4 leveldb: Add more thread safety annotations.
After this CL, all classes with Mutex members should be covered by annotations. Exceptions are atomic members, which shouldn't need locking, and DBImpl members that cause errors when annotated, which will be tackled separately.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=190260865
2018-03-23 12:56:14 -07:00
costan
739c25100e Add CMake build support.
Fixes https://github.com/google/leveldb/issues/466

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=189425354
2018-03-16 19:17:27 -07:00
costan
0fa5a4f7b1 Extend thread safety annotations.
This CL makes it easier to reason about thread safety by:

1) Adding Clang thread safety annotations according to comments.
2) Expanding a couple of variable names, without adding extra lines of code.
3) Adding const in a couple of places.
4) Replacing an always-non-null const pointer with a reference.
5) Fixing style warnings in the modified files.

This CL does not annotate the DBImpl members that claim to be protected
by the instance mutex, but are accessed without the mutex being held.
Those members (and their unprotected accesses) will be addressed in
future CLs.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=189354657
2018-03-16 10:32:40 -07:00
costan
8143c12f3f Fix includes in util/testharness.h.
This CL removes unused headers included by util/testharness.h, adds
precise includes where the build breaks, and fixes style errors in the
edited files.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=189331061
2018-03-16 10:31:48 -07:00
costan
aece2068d7 Remove extern from function declarations.
External linkage is the default for function declarations in C++.

This also fixes ClangTidy errors generated by removing the "extern"
keyword as described above.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=188730416
2018-03-12 09:24:48 -07:00
costan
ddab751002 Add tests for {Old}InfoLogFileName().
This change was contributed by GitHub user @LopatkinEvgeniy in
https://github.com/google/leveldb/pull/559.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=188728845
2018-03-12 09:24:25 -07:00
costan
7fd7c00721 Remove unused function ExtractValueType.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=188728505
2018-03-12 09:24:07 -07:00
costan
623d014a54 Expose Env::GetTempDirectory() for use in C test.
This removes the use of the non-portable headers <sys/types.h> and <unistd.h> in c_test.c.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=188503102
2018-03-09 10:38:04 -08:00
cmumford
47cb9e2a21 Add leveldb_options_set_max_file_size to the C API.
When the max file size option was added in CL 134391640 the C API
was not modified to support this.

This change was contributed by GitHub user @olt and fixes issue #439.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=173466388
2017-11-03 15:04:26 -07:00
cmumford
3da4d8b989 Deleted unused assignments in Reader.
Deleted two unused assignments:

1. offset_in_block in Reader::SkipToInitialBlock().
2. in_fragmented_record in Reader::ReadRecord().

Reasons for the change:
1. offset_in_block is not read again after the if condition.
2. The kFullRecordType switch branch returns, so
   in_fragmented_record isn't read again.
3. The kFirstType switch branch sets in_fragmented_record to
   true after the if, so the write in the if is ignored.

Change contributed by @C0deAi on GitHub.

This fixes https://github.com/google/leveldb/issues/517

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=172763897
2017-11-03 15:03:44 -07:00
cmumford
0509414f85 leveldb::DestroyDB will now delete empty directories.
Env's that filtered out dot files ("." and "..") would return an
empty vector of children causing DestroyDB to do nothing. This fixes
https://github.com/google/leveldb/issues/215

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=172501335
2017-11-03 15:03:20 -07:00
costan
23162ca1c6 Fix typo (forgotten reference operator) in test.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=171708408
2017-10-10 11:47:13 -07:00
sanjay
7e12c00ecf Fix issue 474: a race between the f*_unlocked() STDIO calls in
env_posix.cc and concurrent application calls to fflush(NULL).

The fix is to avoid using stdio in env_posix.cc but add our own
buffering where we need it.

Added a test to reproduce the bug.

Added a test for Env reads/writes.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=170738066
2017-10-03 11:27:09 -07:00
costan
ea0a7586b8 Remove confusing and unnecessary if.
12 lines above, there is an "if (!s.ok()) { return s; }" block of code.
"s" is never modified between that block and the "if" removed by this
CL, so "s.ok()" must be true.

The code most likely intended to say "if (!builder->ok())", because the
builder->Add() call above can modify the TableBuilder's status, as a
side-effect. However, this approach would have required setting "s =
builder.status()" in the "else" branch, near the "builder.Abandon()"
call. So, removing the "if" outright is simpler than following that line
of thought.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167326229
2017-09-01 14:41:28 -07:00