Merge pull request #5325 from sfc-gh-anoyes/anoyes/trace-partial-file-suffix

Add the trace_partial_file_suffix option
This commit is contained in:
A.J. Beamon 2021-08-03 13:56:12 -07:00 committed by GitHub
commit 732a0eda1e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 173 additions and 10 deletions

View File

@ -79,6 +79,7 @@ if(NOT WIN32)
test/unit/fdb_api.hpp)
set(UNIT_TEST_VERSION_510_SRCS test/unit/unit_tests_version_510.cpp)
set(TRACE_PARTIAL_FILE_SUFFIX_TEST_SRCS test/unit/trace_partial_file_suffix_test.cpp)
if(OPEN_FOR_IDE)
add_library(fdb_c_performance_test OBJECT test/performance_test.c test/test.h)
@ -88,6 +89,7 @@ if(NOT WIN32)
add_library(fdb_c_setup_tests OBJECT test/unit/setup_tests.cpp)
add_library(fdb_c_unit_tests OBJECT ${UNIT_TEST_SRCS})
add_library(fdb_c_unit_tests_version_510 OBJECT ${UNIT_TEST_VERSION_510_SRCS})
add_library(trace_partial_file_suffix_test OBJECT ${TRACE_PARTIAL_FILE_SUFFIX_TEST_SRCS})
else()
add_executable(fdb_c_performance_test test/performance_test.c test/test.h)
add_executable(fdb_c_ryw_benchmark test/ryw_benchmark.c test/test.h)
@ -96,6 +98,7 @@ if(NOT WIN32)
add_executable(fdb_c_setup_tests test/unit/setup_tests.cpp)
add_executable(fdb_c_unit_tests ${UNIT_TEST_SRCS})
add_executable(fdb_c_unit_tests_version_510 ${UNIT_TEST_VERSION_510_SRCS})
add_executable(trace_partial_file_suffix_test ${TRACE_PARTIAL_FILE_SUFFIX_TEST_SRCS})
strip_debug_symbols(fdb_c_performance_test)
strip_debug_symbols(fdb_c_ryw_benchmark)
strip_debug_symbols(fdb_c_txn_size_test)
@ -106,12 +109,14 @@ if(NOT WIN32)
add_dependencies(fdb_c_setup_tests doctest)
add_dependencies(fdb_c_unit_tests doctest)
add_dependencies(fdb_c_unit_tests_version_510 doctest)
target_include_directories(fdb_c_setup_tests PUBLIC ${DOCTEST_INCLUDE_DIR})
target_include_directories(fdb_c_unit_tests PUBLIC ${DOCTEST_INCLUDE_DIR})
target_include_directories(fdb_c_unit_tests_version_510 PUBLIC ${DOCTEST_INCLUDE_DIR})
target_link_libraries(fdb_c_setup_tests PRIVATE fdb_c Threads::Threads)
target_link_libraries(fdb_c_unit_tests PRIVATE fdb_c Threads::Threads)
target_link_libraries(fdb_c_unit_tests_version_510 PRIVATE fdb_c Threads::Threads)
target_link_libraries(trace_partial_file_suffix_test PRIVATE fdb_c Threads::Threads)
# do not set RPATH for mako
set_property(TARGET mako PROPERTY SKIP_BUILD_RPATH TRUE)
@ -146,6 +151,11 @@ if(NOT WIN32)
COMMAND $<TARGET_FILE:fdb_c_unit_tests_version_510>
@CLUSTER_FILE@
fdb)
add_fdbclient_test(
NAME trace_partial_file_suffix_test
COMMAND $<TARGET_FILE:trace_partial_file_suffix_test>
@CLUSTER_FILE@
fdb)
add_fdbclient_test(
NAME fdb_c_external_client_unit_tests
COMMAND $<TARGET_FILE:fdb_c_unit_tests>

View File

@ -0,0 +1,111 @@
/*
* trace_partial_file_suffix_test.cpp
*
* This source file is part of the FoundationDB open source project
*
* Copyright 2013-2021 Apple Inc. and the FoundationDB project authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <fstream>
#include <iostream>
#include <random>
#include <string>
#include <thread>
#include "flow/Platform.h"
#define FDB_API_VERSION 710
#include "foundationdb/fdb_c.h"
#undef NDEBUG
#include <cassert>
void fdb_check(fdb_error_t e) {
if (e) {
std::cerr << fdb_get_error(e) << std::endl;
std::abort();
}
}
void set_net_opt(FDBNetworkOption option, const std::string& value) {
fdb_check(fdb_network_set_option(option, reinterpret_cast<const uint8_t*>(value.c_str()), value.size()));
}
bool file_exists(const char* path) {
FILE* f = fopen(path, "r");
if (f) {
fclose(f);
return true;
}
return false;
}
int main(int argc, char** argv) {
fdb_check(fdb_select_api_version(710));
std::string file_identifier = "trace_partial_file_suffix_test" + std::to_string(std::random_device{}());
std::string trace_partial_file_suffix = ".tmp";
std::string simulated_stray_partial_file =
"trace.127.0.0.1." + file_identifier + ".simulated.xml" + trace_partial_file_suffix;
// Simulate this process crashing previously by creating a ".tmp" file
{ std::ofstream file{ simulated_stray_partial_file }; }
set_net_opt(FDBNetworkOption::FDB_NET_OPTION_TRACE_ENABLE, "");
set_net_opt(FDBNetworkOption::FDB_NET_OPTION_TRACE_FILE_IDENTIFIER, file_identifier);
set_net_opt(FDBNetworkOption::FDB_NET_OPTION_TRACE_PARTIAL_FILE_SUFFIX, trace_partial_file_suffix);
fdb_check(fdb_setup_network());
std::thread network_thread{ &fdb_run_network };
// Apparently you need to open a database to initialize logging
FDBDatabase* out;
fdb_check(fdb_create_database(nullptr, &out));
fdb_database_destroy(out);
// Eventually there's a new trace file for this test ending in .tmp
std::string name;
for (;;) {
for (const auto& path : platform::listFiles(".")) {
if (path.find(file_identifier) != std::string::npos && path.find(".simulated.") == std::string::npos) {
assert(path.substr(path.size() - trace_partial_file_suffix.size()) == trace_partial_file_suffix);
name = path;
break;
}
}
if (!name.empty()) {
break;
}
}
fdb_check(fdb_stop_network());
network_thread.join();
// After shutting down, the suffix is removed for both the simulated stray file and our new file
if (!trace_partial_file_suffix.empty()) {
assert(!file_exists(name.c_str()));
assert(!file_exists(simulated_stray_partial_file.c_str()));
}
auto new_name = name.substr(0, name.size() - trace_partial_file_suffix.size());
auto new_stray_name =
simulated_stray_partial_file.substr(0, simulated_stray_partial_file.size() - trace_partial_file_suffix.size());
assert(file_exists(new_name.c_str()));
assert(file_exists(new_stray_name.c_str()));
remove(new_name.c_str());
remove(new_stray_name.c_str());
assert(!file_exists(new_name.c_str()));
assert(!file_exists(new_stray_name.c_str()));
}

View File

@ -1698,7 +1698,8 @@ Database Database::createDatabase(Reference<ClusterConnectionFile> connFile,
networkOptions.traceDirectory.get(),
"trace",
networkOptions.traceLogGroup,
networkOptions.traceFileIdentifier);
networkOptions.traceFileIdentifier,
networkOptions.tracePartialFileSuffix);
TraceEvent("ClientStart")
.detail("SourceVersion", getSourceVersion())
@ -1856,6 +1857,10 @@ void setNetworkOption(FDBNetworkOptions::Option option, Optional<StringRef> valu
throw invalid_option_value();
}
break;
case FDBNetworkOptions::TRACE_PARTIAL_FILE_SUFFIX:
validateOptionValuePresent(value);
networkOptions.tracePartialFileSuffix = value.get().toString();
break;
case FDBNetworkOptions::KNOB: {
validateOptionValuePresent(value);

View File

@ -68,6 +68,7 @@ struct NetworkOptions {
std::string traceFormat;
std::string traceClockSource;
std::string traceFileIdentifier;
std::string tracePartialFileSuffix;
Optional<bool> logClientInfo;
Reference<ReferencedObject<Standalone<VectorRef<ClientVersionRef>>>> supportedVersions;
bool runLoopProfilingEnabled;

View File

@ -57,6 +57,9 @@ description is not currently required but encouraged.
<Option name="trace_file_identifier" code="36"
paramType="String" paramDescription="The identifier that will be part of all trace file names"
description="Once provided, this string will be used to replace the port/PID in the log file names." />
<Option name="trace_partial_file_suffix" code="39"
paramType="String" paramDesciption="Append this suffix to partially written log files. When a log file is complete, it is renamed to remove the suffix. No separator is added between the file and the suffix. If you want to add a file extension, you should include the separator - e.g. '.tmp' instead of 'tmp' to add the 'tmp' extension."
description="" />
<Option name="knob" code="40"
paramType="String" paramDescription="knob_name=knob_value"
description="Set internal tuning or debugging knobs"/>

View File

@ -19,6 +19,7 @@
*/
#include "flow/FileTraceLogWriter.h"
#include "flow/Platform.h"
#include "flow/flow.h"
#include "flow/ThreadHelper.actor.h"
@ -87,11 +88,13 @@ FileTraceLogWriter::FileTraceLogWriter(std::string const& directory,
std::string const& processName,
std::string const& basename,
std::string const& extension,
std::string const& tracePartialFileSuffix,
uint64_t maxLogsSize,
std::function<void()> const& onError,
Reference<ITraceLogIssuesReporter> const& issues)
: directory(directory), processName(processName), basename(basename), extension(extension), maxLogsSize(maxLogsSize),
traceFileFD(-1), index(0), issues(issues), onError(onError) {}
: directory(directory), processName(processName), basename(basename), extension(extension),
tracePartialFileSuffix(tracePartialFileSuffix), maxLogsSize(maxLogsSize), traceFileFD(-1), index(0), issues(issues),
onError(onError) {}
void FileTraceLogWriter::addref() {
ReferenceCounted<FileTraceLogWriter>::addref();
@ -158,7 +161,8 @@ void FileTraceLogWriter::open() {
// log10(index) < 10
UNSTOPPABLE_ASSERT(indexWidth < 10);
auto finalname = format("%s.%d.%d.%s", basename.c_str(), indexWidth, index, extension.c_str());
finalname =
format("%s.%d.%d.%s%s", basename.c_str(), indexWidth, index, extension.c_str(), tracePartialFileSuffix.c_str());
while ((traceFileFD = __open(finalname.c_str(), TRACEFILE_FLAGS, TRACEFILE_MODE)) == -1) {
lastError(errno);
if (errno == EEXIST) {
@ -166,7 +170,12 @@ void FileTraceLogWriter::open() {
indexWidth = unsigned(::floor(log10f(float(index))));
UNSTOPPABLE_ASSERT(indexWidth < 10);
finalname = format("%s.%d.%d.%s", basename.c_str(), indexWidth, index, extension.c_str());
finalname = format("%s.%d.%d.%s%s",
basename.c_str(),
indexWidth,
index,
extension.c_str(),
tracePartialFileSuffix.c_str());
} else {
fprintf(stderr,
"ERROR: could not create trace log file `%s' (%d: %s)\n",
@ -178,7 +187,7 @@ void FileTraceLogWriter::open() {
int errorNum = errno;
onMainThreadVoid(
[finalname, errorNum] {
[finalname = finalname, errorNum] {
TraceEvent(SevWarnAlways, "TraceFileOpenError")
.detail("Filename", finalname)
.detail("ErrorCode", errorNum)
@ -201,6 +210,11 @@ void FileTraceLogWriter::close() {
while (__close(traceFileFD))
threadSleep(0.1);
}
traceFileFD = -1;
if (!tracePartialFileSuffix.empty()) {
renameFile(finalname, finalname.substr(0, finalname.size() - tracePartialFileSuffix.size()));
}
finalname = "";
}
void FileTraceLogWriter::roll() {
@ -216,6 +230,15 @@ void FileTraceLogWriter::cleanupTraceFiles() {
// Setting maxLogsSize=0 disables trace file cleanup based on dir size
if (!g_network->isSimulated() && maxLogsSize > 0) {
try {
// Rename/finalize any stray files ending in tracePartialFileSuffix for this process.
if (!tracePartialFileSuffix.empty()) {
for (const auto& f : platform::listFiles(directory, tracePartialFileSuffix)) {
if (f.substr(0, processName.length()) == processName) {
renameFile(f, f.substr(0, f.size() - tracePartialFileSuffix.size()));
}
}
}
std::vector<std::string> existingFiles = platform::listFiles(directory, extension);
std::vector<std::string> existingTraceFiles;

View File

@ -51,6 +51,8 @@ private:
std::string processName;
std::string basename;
std::string extension;
std::string finalname;
std::string tracePartialFileSuffix;
uint64_t maxLogsSize;
int traceFileFD;
@ -66,6 +68,7 @@ public:
std::string const& processName,
std::string const& basename,
std::string const& extension,
std::string const& tracePartialFileSuffix,
uint64_t maxLogsSize,
std::function<void()> const& onError,
Reference<ITraceLogIssuesReporter> const& issues);

View File

@ -115,6 +115,7 @@ private:
std::string directory;
std::string processName;
Optional<NetworkAddress> localAddress;
std::string tracePartialFileSuffix;
Reference<IThreadPool> writer;
uint64_t rollsize;
@ -288,13 +289,15 @@ public:
std::string const& timestamp,
uint64_t rs,
uint64_t maxLogsSize,
Optional<NetworkAddress> na) {
Optional<NetworkAddress> na,
std::string const& tracePartialFileSuffix) {
ASSERT(!writer && !opened);
this->directory = directory;
this->processName = processName;
this->logGroup = logGroup;
this->localAddress = na;
this->tracePartialFileSuffix = tracePartialFileSuffix;
basename = format("%s/%s.%s.%s",
directory.c_str(),
@ -306,6 +309,7 @@ public:
processName,
basename,
formatter->getExtension(),
tracePartialFileSuffix,
maxLogsSize,
[this]() { barriers->triggerAll(); },
issues));
@ -715,7 +719,8 @@ void openTraceFile(const NetworkAddress& na,
std::string directory,
std::string baseOfBase,
std::string logGroup,
std::string identifier) {
std::string identifier,
std::string tracePartialFileSuffix) {
if (g_traceLog.isOpen())
return;
@ -739,7 +744,8 @@ void openTraceFile(const NetworkAddress& na,
format("%lld", time(nullptr)),
rollsize,
maxLogsSize,
!g_network->isSimulated() ? na : Optional<NetworkAddress>());
!g_network->isSimulated() ? na : Optional<NetworkAddress>(),
tracePartialFileSuffix);
uncancellable(recurring(&flushTraceFile, FLOW_KNOBS->TRACE_FLUSH_INTERVAL, TaskPriority::FlushTrace));
g_traceBatch.dump();

View File

@ -564,7 +564,8 @@ void openTraceFile(const NetworkAddress& na,
std::string directory = ".",
std::string baseOfBase = "trace",
std::string logGroup = "default",
std::string identifier = "");
std::string identifier = "",
std::string tracePartialFileSuffix = "");
void initTraceEventMetrics();
void closeTraceFile();
bool traceFileIsOpen();