Merge pull request #1464 from atn34/review-comments

Address review comments from #1446
This commit is contained in:
A.J. Beamon 2019-04-16 14:11:24 -07:00 committed by GitHub
commit 6327c78bac
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 19 additions and 17 deletions

View File

@ -20,5 +20,5 @@
#include "fdbrpc/AsyncFileWriteChecker.h"
int AsyncFileWriteChecker::checksumHistoryBudget;
Optional<int> AsyncFileWriteChecker::checksumHistoryBudget = {};
int AsyncFileWriteChecker::checksumHistoryPageSize = 4096;

View File

@ -48,7 +48,7 @@ public:
if( (size / checksumHistoryPageSize) < checksumHistory.size() ) {
int oldCapacity = checksumHistory.capacity();
checksumHistory.resize(size / checksumHistoryPageSize);
checksumHistoryBudget -= (checksumHistory.capacity() - oldCapacity);
checksumHistoryBudget.get() -= (checksumHistory.capacity() - oldCapacity);
}
return r;
});
@ -63,17 +63,15 @@ public:
AsyncFileWriteChecker(Reference<IAsyncFile> f) : m_f(f) {
// Initialize the static history budget the first time (and only the first time) a file is opened.
static int _ = checksumHistoryBudget = FLOW_KNOBS->PAGE_WRITE_CHECKSUM_HISTORY;
(void)_;
if (!checksumHistoryBudget.present()) {
checksumHistoryBudget = FLOW_KNOBS->PAGE_WRITE_CHECKSUM_HISTORY;
}
// Adjust the budget by the initial capacity of history, which should be 0 but maybe not for some implementations.
checksumHistoryBudget -= checksumHistory.capacity();
checksumHistoryBudget.get() -= checksumHistory.capacity();
}
virtual ~AsyncFileWriteChecker() {
checksumHistoryBudget += checksumHistory.capacity();
}
virtual ~AsyncFileWriteChecker() { checksumHistoryBudget.get() += checksumHistory.capacity(); }
private:
Reference<IAsyncFile> m_f;
@ -86,7 +84,7 @@ private:
std::vector<WriteInfo> checksumHistory;
// This is the most page checksum history blocks we will use across all files.
static int checksumHistoryBudget;
static Optional<int> checksumHistoryBudget;
static int checksumHistoryPageSize;
// Update or check checksum(s) in history for any full pages covered by this operation
@ -104,11 +102,12 @@ private:
// Make sure history is large enough or limit pageEnd
if(checksumHistory.size() < pageEnd) {
if(checksumHistoryBudget > 0) {
if (checksumHistoryBudget.get() > 0) {
// Resize history and update budget based on capacity change
auto initialCapacity = checksumHistory.capacity();
checksumHistory.resize(checksumHistory.size() + std::min<int>(checksumHistoryBudget, pageEnd - checksumHistory.size()));
checksumHistoryBudget -= (checksumHistory.capacity() - initialCapacity);
checksumHistory.resize(checksumHistory.size() +
std::min<int>(checksumHistoryBudget.get(), pageEnd - checksumHistory.size()));
checksumHistoryBudget.get() -= (checksumHistory.capacity() - initialCapacity);
}
// Limit pageEnd to end of history, which works whether or not all of the desired

View File

@ -265,6 +265,7 @@ typedef WorkPool<Coroutine, ThreadUnsafeSpinLock, true> CoroPool;
ACTOR void coroSwitcher( Future<Void> what, int taskID, Coro* coro ) {
try {
// state double t = now();
wait(what);
//if (g_network->isSimulated() && g_simulator.getCurrentProcess()->rebooting && now()!=t)
// TraceEvent("NonzeroWaitDuringReboot").detail("TaskID", taskID).detail("Elapsed", now()-t).backtrace("Flow");
@ -278,6 +279,7 @@ ACTOR void coroSwitcher( Future<Void> what, int taskID, Coro* coro ) {
void CoroThreadPool::waitFor( Future<Void> what ) {
ASSERT (current_coro != main_coro);
if (what.isReady()) return;
// double t = now();
coroSwitcher(what, g_network->getCurrentTask(), current_coro);
Coro_switchTo_( swapCoro(main_coro), main_coro );
//if (g_network->isSimulated() && g_simulator.getCurrentProcess()->rebooting && now()!=t)

View File

@ -229,6 +229,7 @@ struct VersionStampWorkload : TestWorkload {
auto last_element_iter = all_values.cend(); last_element_iter--;
ASSERT(value_pair_iter == last_element_iter);
}
// Version commitVersion = value_pair_iter->first;
Standalone<StringRef> commitVersionstamp = value_pair_iter->second;
//TraceEvent("VST_Check0b").detail("Version", commitVersion).detail("CommitVersion", printable(commitVersionstamp));
@ -266,6 +267,7 @@ struct VersionStampWorkload : TestWorkload {
ASSERT(value_pair_iter == last_element_iter);
}
// Version commitVersion = value_pair_iter->first;
Standalone<StringRef> commitVersionstamp = value_pair_iter->second;
//TraceEvent("VST_Check1b").detail("Version", commitVersion).detail("CommitVersion", printable(commitVersionstamp));
ASSERT(parsedVersion <= readVersion);

View File

@ -396,7 +396,7 @@ TEST_CASE("/flow/IndexedSet/all numbers") {
for (int i = 0; i<100000; i++) {
int b = g_random->randomInt(1, (int)allNumbers.size());
int64_t ntotal = int64_t(b)*(b - 1) / 2;
int64_t n = ntotal;// + g_random->randomInt( 0, int(std::max<int64_t>(1<<30,nmax-ntotal)) );
int64_t n = ntotal;
auto ii = is.index(n);
int ib = ii != is.end() ? *ii : 1000000;
ASSERT(ib == b);

View File

@ -50,9 +50,8 @@
using namespace std::rel_ops;
#define TEST(condition) \
if (!(condition)) \
; \
else { \
if (!(condition)) { \
} else { \
static TraceEvent* __test = &(TraceEvent("CodeCoverage") \
.detail("File", __FILE__) \
.detail("Line", __LINE__) \