Skip to content

Commit 2fef013

Browse files
committed
Fix a bug for surfacing write unix time (#13057)
Summary: The write unix time from non L0 files are not surfaced properly because the level's wrapper iterator doesn't have a `write_unix_time` implementation that delegates to the corresponding file. The unit test didn't catch this because it incorrectly destroy the old db and reopen to check write time, instead of just reopen and check. This fix also include a change to support ldb's scan command to get write time for easier debugging. Pull Request resolved: #13057 Test Plan: Updated unit tests Reviewed By: pdillinger Differential Revision: D64015107 Pulled By: jowlyzhang fbshipit-source-id: 244474f78a034f80c9235eea2aa8a0f4e54dff59
1 parent a245672 commit 2fef013

File tree

7 files changed

+49
-7
lines changed

7 files changed

+49
-7
lines changed

db/compaction/tiered_compaction_test.cc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2512,6 +2512,7 @@ TEST_P(IteratorWriteTimeTest, ReadFromMemtables) {
25122512
start_time + kSecondsPerRecording * (i + 1));
25132513
}
25142514
}
2515+
ASSERT_EQ(kNumKeys, i);
25152516
ASSERT_OK(iter->status());
25162517
}
25172518

@@ -2531,12 +2532,13 @@ TEST_P(IteratorWriteTimeTest, ReadFromMemtables) {
25312532
}
25322533
}
25332534
ASSERT_OK(iter->status());
2535+
ASSERT_EQ(-1, i);
25342536
}
25352537

25362538
// Reopen the DB and disable the seqno to time recording, data with user
25372539
// specified write time can still get a write time before it's flushed.
25382540
options.preserve_internal_time_seconds = 0;
2539-
DestroyAndReopen(options);
2541+
Reopen(options);
25402542
ASSERT_OK(TimedPut(Key(kKeyWithWriteTime), rnd.RandomString(100),
25412543
kUserSpecifiedWriteTime));
25422544
{
@@ -2613,6 +2615,7 @@ TEST_P(IteratorWriteTimeTest, ReadFromSstFile) {
26132615
}
26142616
}
26152617
ASSERT_OK(iter->status());
2618+
ASSERT_EQ(kNumKeys, i);
26162619
}
26172620

26182621
// Backward iteration
@@ -2632,12 +2635,13 @@ TEST_P(IteratorWriteTimeTest, ReadFromSstFile) {
26322635
}
26332636
}
26342637
ASSERT_OK(iter->status());
2638+
ASSERT_EQ(-1, i);
26352639
}
26362640

26372641
// Reopen the DB and disable the seqno to time recording. Data retrieved from
26382642
// SST files still have write time available.
26392643
options.preserve_internal_time_seconds = 0;
2640-
DestroyAndReopen(options);
2644+
Reopen(options);
26412645

26422646
dbfull()->TEST_WaitForPeriodicTaskRun(
26432647
[&] { mock_clock_->MockSleepForSeconds(kSecondsPerRecording); });
@@ -2663,6 +2667,7 @@ TEST_P(IteratorWriteTimeTest, ReadFromSstFile) {
26632667
start_time + kSecondsPerRecording * (i + 1));
26642668
}
26652669
}
2670+
ASSERT_EQ(kNumKeys, i);
26662671
ASSERT_OK(iter->status());
26672672
}
26682673

@@ -2686,6 +2691,7 @@ TEST_P(IteratorWriteTimeTest, ReadFromSstFile) {
26862691
VerifyKeyAndWriteTime(iter.get(), Key(i), 0);
26872692
}
26882693
ASSERT_OK(iter->status());
2694+
ASSERT_EQ(kNumKeys, i);
26892695
}
26902696
Close();
26912697
}

db/forward_iterator.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,10 @@ class ForwardLevelIterator : public InternalIterator {
167167
assert(valid_);
168168
return file_iter_->value();
169169
}
170+
uint64_t write_unix_time() const override {
171+
assert(valid_);
172+
return file_iter_->write_unix_time();
173+
}
170174
Status status() const override {
171175
if (!status_.ok()) {
172176
return status_;

db/version_set.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,6 +1050,11 @@ class LevelIterator final : public InternalIterator {
10501050
return file_iter_.value();
10511051
}
10521052

1053+
uint64_t write_unix_time() const override {
1054+
assert(Valid());
1055+
return file_iter_.write_unix_time();
1056+
}
1057+
10531058
Status status() const override {
10541059
return file_iter_.iter() ? file_iter_.status() : Status::OK();
10551060
}

include/rocksdb/utilities/ldb_cmd.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ class LDBCommand {
7474
static const std::string ARG_DECODE_BLOB_INDEX;
7575
static const std::string ARG_DUMP_UNCOMPRESSED_BLOBS;
7676
static const std::string ARG_READ_TIMESTAMP;
77+
static const std::string ARG_GET_WRITE_UNIX_TIME;
7778

7879
struct ParsedParams {
7980
std::string cmd;

table/iterator.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ class EmptyInternalIterator : public InternalIteratorBase<TValue> {
7474
assert(false);
7575
return TValue();
7676
}
77+
uint64_t write_unix_time() const override {
78+
assert(false);
79+
return std::numeric_limits<uint64_t>::max();
80+
}
7781
Status status() const override { return status_; }
7882

7983
private:

tools/ldb_cmd.cc

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "db/write_batch_internal.h"
2828
#include "file/filename.h"
2929
#include "rocksdb/cache.h"
30+
#include "rocksdb/comparator.h"
3031
#include "rocksdb/experimental.h"
3132
#include "rocksdb/file_checksum.h"
3233
#include "rocksdb/filter_policy.h"
@@ -110,6 +111,7 @@ const std::string LDBCommand::ARG_DECODE_BLOB_INDEX = "decode_blob_index";
110111
const std::string LDBCommand::ARG_DUMP_UNCOMPRESSED_BLOBS =
111112
"dump_uncompressed_blobs";
112113
const std::string LDBCommand::ARG_READ_TIMESTAMP = "read_timestamp";
114+
const std::string LDBCommand::ARG_GET_WRITE_UNIX_TIME = "get_write_unix_time";
113115

114116
const char* LDBCommand::DELIM = " ==> ";
115117

@@ -3622,11 +3624,12 @@ void BatchPutCommand::OverrideBaseOptions() {
36223624
ScanCommand::ScanCommand(const std::vector<std::string>& /*params*/,
36233625
const std::map<std::string, std::string>& options,
36243626
const std::vector<std::string>& flags)
3625-
: LDBCommand(options, flags, true,
3626-
BuildCmdLineOptions(
3627-
{ARG_TTL, ARG_NO_VALUE, ARG_HEX, ARG_KEY_HEX, ARG_TO,
3628-
ARG_VALUE_HEX, ARG_FROM, ARG_TIMESTAMP, ARG_MAX_KEYS,
3629-
ARG_TTL_START, ARG_TTL_END, ARG_READ_TIMESTAMP})),
3627+
: LDBCommand(
3628+
options, flags, true,
3629+
BuildCmdLineOptions({ARG_TTL, ARG_NO_VALUE, ARG_HEX, ARG_KEY_HEX,
3630+
ARG_TO, ARG_VALUE_HEX, ARG_FROM, ARG_TIMESTAMP,
3631+
ARG_MAX_KEYS, ARG_TTL_START, ARG_TTL_END,
3632+
ARG_READ_TIMESTAMP, ARG_GET_WRITE_UNIX_TIME})),
36303633
start_key_specified_(false),
36313634
end_key_specified_(false),
36323635
max_keys_scanned_(-1),
@@ -3670,6 +3673,7 @@ ScanCommand::ScanCommand(const std::vector<std::string>& /*params*/,
36703673
ARG_MAX_KEYS + " has a value out-of-range");
36713674
}
36723675
}
3676+
get_write_unix_time_ = IsFlagPresent(flags_, ARG_GET_WRITE_UNIX_TIME);
36733677
}
36743678

36753679
void ScanCommand::Help(std::string& ret) {
@@ -3683,6 +3687,7 @@ void ScanCommand::Help(std::string& ret) {
36833687
ret.append(" [--" + ARG_TTL_END + "=<N>:- is exclusive]");
36843688
ret.append(" [--" + ARG_NO_VALUE + "]");
36853689
ret.append(" [--" + ARG_READ_TIMESTAMP + "=<uint64_ts>] ");
3690+
ret.append(" [--" + ARG_GET_WRITE_UNIX_TIME + "]");
36863691
ret.append("\n");
36873692
}
36883693

@@ -3765,6 +3770,22 @@ void ScanCommand::DoCommand() {
37653770
fprintf(stdout, "%s\n", str.c_str());
37663771
}
37673772

3773+
if (get_write_unix_time_) {
3774+
std::string write_unix_time;
3775+
uint64_t write_time_int = std::numeric_limits<uint64_t>::max();
3776+
Status s =
3777+
it->GetProperty("rocksdb.iterator.write-time", &write_unix_time);
3778+
if (s.ok()) {
3779+
s = DecodeU64Ts(write_unix_time, &write_time_int);
3780+
}
3781+
if (!s.ok()) {
3782+
fprintf(stdout, " Failed to get write unix time: %s\n",
3783+
s.ToString().c_str());
3784+
} else {
3785+
fprintf(stdout, " write unix time: %s\n",
3786+
std::to_string(write_time_int).c_str());
3787+
}
3788+
}
37683789
num_keys_scanned++;
37693790
if (max_keys_scanned_ >= 0 && num_keys_scanned >= max_keys_scanned_) {
37703791
break;

tools/ldb_cmd_impl.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,7 @@ class ScanCommand : public LDBCommand {
511511
bool end_key_specified_;
512512
int max_keys_scanned_;
513513
bool no_value_;
514+
bool get_write_unix_time_;
514515
};
515516

516517
class DeleteCommand : public LDBCommand {

0 commit comments

Comments
 (0)