Fix GetCurrentTime() initialization for valgrind
Summary: Valgrind had false positive complaints about the initialization pattern for `GetCurrentTime()`'s argument in #2480. We can instead have the client initialize the time variable before calling `GetCurrentTime()`, and have `GetCurrentTime()` promise to only overwrite it in success case. Closes https://github.com/facebook/rocksdb/pull/2526 Differential Revision: D5358689 Pulled By: ajkr fbshipit-source-id: 857b189f24c19196f6bb299216f3e23e7bc4be42
This commit is contained in:
parent
f6b9d9355e
commit
33042573db
@ -1283,11 +1283,8 @@ Status CompactionJob::OpenCompactionOutputFile(
|
||||
uint64_t output_file_creation_time =
|
||||
sub_compact->compaction->MaxInputFileCreationTime();
|
||||
if (output_file_creation_time == 0) {
|
||||
int64_t _current_time;
|
||||
auto status = db_options_.env->GetCurrentTime(&_current_time);
|
||||
if (!status.ok()) {
|
||||
_current_time = 0;
|
||||
}
|
||||
int64_t _current_time = 0;
|
||||
db_options_.env->GetCurrentTime(&_current_time); // ignore error
|
||||
output_file_creation_time = static_cast<uint64_t>(_current_time);
|
||||
}
|
||||
|
||||
|
@ -854,11 +854,8 @@ Status DBImpl::WriteLevel0TableForRecovery(int job_id, ColumnFamilyData* cfd,
|
||||
bool paranoid_file_checks =
|
||||
cfd->GetLatestMutableCFOptions()->paranoid_file_checks;
|
||||
|
||||
int64_t _current_time;
|
||||
s = env_->GetCurrentTime(&_current_time);
|
||||
if (!s.ok()) {
|
||||
_current_time = 0;
|
||||
}
|
||||
int64_t _current_time = 0;
|
||||
env_->GetCurrentTime(&_current_time); // ignore error
|
||||
const uint64_t current_time = static_cast<uint64_t>(_current_time);
|
||||
|
||||
{
|
||||
|
@ -299,11 +299,8 @@ Status FlushJob::WriteLevel0Table() {
|
||||
EnvOptions optimized_env_options =
|
||||
db_options_.env->OptimizeForCompactionTableWrite(env_options_, db_options_);
|
||||
|
||||
int64_t _current_time;
|
||||
auto status = db_options_.env->GetCurrentTime(&_current_time);
|
||||
if (!status.ok()) {
|
||||
_current_time = 0;
|
||||
}
|
||||
int64_t _current_time = 0;
|
||||
db_options_.env->GetCurrentTime(&_current_time); // ignore error
|
||||
const uint64_t current_time = static_cast<uint64_t>(_current_time);
|
||||
|
||||
s = BuildTable(
|
||||
|
@ -383,11 +383,8 @@ class Repairer {
|
||||
EnvOptions optimized_env_options =
|
||||
env_->OptimizeForCompactionTableWrite(env_options_, immutable_db_options_);
|
||||
|
||||
int64_t _current_time;
|
||||
status = env_->GetCurrentTime(&_current_time);
|
||||
if (!status.ok()) {
|
||||
_current_time = 0;
|
||||
}
|
||||
int64_t _current_time = 0;
|
||||
status = env_->GetCurrentTime(&_current_time); // ignore error
|
||||
const uint64_t current_time = static_cast<uint64_t>(_current_time);
|
||||
|
||||
status = BuildTable(
|
||||
|
6
env/mock_env.cc
vendored
6
env/mock_env.cc
vendored
@ -148,7 +148,7 @@ class MemFile {
|
||||
|
||||
private:
|
||||
uint64_t Now() {
|
||||
int64_t unix_time;
|
||||
int64_t unix_time = 0;
|
||||
auto s = env_->GetCurrentTime(&unix_time);
|
||||
assert(s.ok());
|
||||
return static_cast<uint64_t>(unix_time);
|
||||
@ -734,7 +734,9 @@ Status MockEnv::GetTestDirectory(std::string* path) {
|
||||
|
||||
Status MockEnv::GetCurrentTime(int64_t* unix_time) {
|
||||
auto s = EnvWrapper::GetCurrentTime(unix_time);
|
||||
*unix_time += fake_sleep_micros_.load() / (1000 * 1000);
|
||||
if (s.ok()) {
|
||||
*unix_time += fake_sleep_micros_.load() / (1000 * 1000);
|
||||
}
|
||||
return s;
|
||||
}
|
||||
|
||||
|
@ -353,6 +353,7 @@ class Env {
|
||||
virtual Status GetHostName(char* name, uint64_t len) = 0;
|
||||
|
||||
// Get the number of seconds since the Epoch, 1970-01-01 00:00:00 (UTC).
|
||||
// Only overwrites *unix_time on success.
|
||||
virtual Status GetCurrentTime(int64_t* unix_time) = 0;
|
||||
|
||||
// Get full directory name for this db.
|
||||
|
@ -27,7 +27,7 @@ bool DbDumpTool::Run(const DumpOptions& dump_options,
|
||||
rocksdb::Status status;
|
||||
std::unique_ptr<rocksdb::WritableFile> dumpfile;
|
||||
char hostname[1024];
|
||||
int64_t timesec;
|
||||
int64_t timesec = 0;
|
||||
std::string abspath;
|
||||
char json[4096];
|
||||
|
||||
|
@ -37,7 +37,7 @@ class SpecialTimeEnv : public EnvWrapper {
|
||||
}
|
||||
|
||||
private:
|
||||
int64_t current_time_;
|
||||
int64_t current_time_ = 0;
|
||||
};
|
||||
|
||||
class DateTieredTest : public testing::Test {
|
||||
|
@ -36,7 +36,7 @@ class SpecialTimeEnv : public EnvWrapper {
|
||||
}
|
||||
|
||||
private:
|
||||
int64_t current_time_;
|
||||
int64_t current_time_ = 0;
|
||||
};
|
||||
|
||||
class TtlTest : public testing::Test {
|
||||
|
Loading…
Reference in New Issue
Block a user