Skip to content

Commit 5a09d79

Browse files
authored
Adding a new FSM token and running FSMLeakChecker for all e2e tests (#6002)
* Refactor FSMLeakChecker * Adding a new SKIP_FSM_LEAK_CHECK token to e2e tests * Avoid flushing empty column chunks * Run the FSMLeakChecker for all e2e tests. * Skip the FSM LEAK CHECK for certain tests to avoid checkpoint error. * ci: auto code format --------- Co-authored-by: CI Bot <[email protected]>
1 parent 2f52795 commit 5a09d79

File tree

13 files changed

+126
-55
lines changed

13 files changed

+126
-55
lines changed

src/storage/table/column_chunk_data.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,7 @@ void ColumnChunkData::setToOnDisk(const ColumnChunkMetadata& otherMetadata) {
324324

325325
ColumnChunkMetadata ColumnChunkData::flushBuffer(PageAllocator& pageAllocator,
326326
const PageRange& entry, const ColumnChunkMetadata& otherMetadata) const {
327-
if (!otherMetadata.compMeta.isConstant() && getBufferSize() != 0) {
328-
KU_ASSERT(getBufferSize() == getBufferSize(capacity));
327+
if (!otherMetadata.compMeta.isConstant() && getBufferSize(numValues) != 0) {
329328
return flushBufferFunction(buffer->getBuffer(), pageAllocator.getDataFH(), entry,
330329
otherMetadata);
331330
}

test/copy/copy_test.cpp

Lines changed: 1 addition & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "storage/buffer_manager/buffer_manager.h"
66
#include "storage/checkpointer.h"
77
#include "storage/storage_manager.h"
8+
#include "test_runner/fsm_leak_checker.h"
89
#include "test_runner/test_parser.h"
910
#include "transaction/transaction_manager.h"
1011

@@ -114,57 +115,6 @@ class CopyTest : public BaseGraphTest {
114115
FlakyBufferManager* currentBM;
115116
};
116117

117-
static decltype(auto) getDbSizeInPages(main::Connection* conn) {
118-
return storage::StorageManager::Get(*conn->getClientContext())->getDataFH()->getNumPages();
119-
}
120-
121-
static decltype(auto) getNumFreePages(main::Connection* conn) {
122-
auto& pageManager = *storage::PageManager::Get(*conn->getClientContext());
123-
auto numFreeEntries = pageManager.getNumFreeEntries();
124-
auto entries = pageManager.getFreeEntries(0, numFreeEntries);
125-
return std::accumulate(entries.begin(), entries.end(), 0ull,
126-
[](auto a, auto b) { return a + b.numPages; });
127-
}
128-
129-
struct FSMLeakChecker {
130-
static void checkForLeakedPages(main::Connection* conn) {
131-
conn->query("checkpoint");
132-
133-
std::vector<std::pair<std::string, std::string>> tableNames;
134-
auto tableNamesResult = conn->query("call show_tables() return name, type");
135-
while (tableNamesResult->hasNext()) {
136-
auto nextResult = tableNamesResult->getNext();
137-
tableNames.emplace_back(nextResult->getValue(0)->toString(),
138-
nextResult->getValue(1)->toString());
139-
}
140-
141-
// Drop rel tables first
142-
for (const auto& [tableName, tableType] : tableNames) {
143-
if (tableType == common::TableTypeUtils::toString(common::TableType::REL)) {
144-
ASSERT_TRUE(
145-
conn->query(common::stringFormat("drop table {}", tableName))->isSuccess());
146-
}
147-
}
148-
for (const auto& [tableName, tableType] : tableNames) {
149-
if (tableType != common::TableTypeUtils::toString(common::TableType::REL)) {
150-
ASSERT_TRUE(
151-
conn->query(common::stringFormat("drop table {}", tableName))->isSuccess());
152-
}
153-
}
154-
conn->query("checkpoint");
155-
156-
const auto numTotalPages = getDbSizeInPages(conn);
157-
const auto numUsedPages = numTotalPages - getNumFreePages(conn);
158-
159-
storage::Checkpointer checkpointer{*conn->getClientContext()};
160-
auto databaseHeader = storage::StorageManager::Get(*conn->getClientContext())
161-
->getOrInitDatabaseHeader(*conn->getClientContext());
162-
ASSERT_EQ(1 + databaseHeader->catalogPageRange.numPages +
163-
databaseHeader->metadataPageRange.numPages,
164-
numUsedPages);
165-
}
166-
};
167-
168118
void CopyTest::BMExceptionRecoveryTest(BMExceptionRecoveryTestConfig cfg) {
169119
if (inMemMode) {
170120
failureFrequency = UINT64_MAX;

test/graph_test/private_graph_test.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "graph_test/base_graph_test.h"
55
#include "spdlog/spdlog.h"
66
#include "storage/storage_manager.h"
7+
#include "test_runner/fsm_leak_checker.h"
78
#include "test_runner/insert_by_row.h"
89
#include "test_runner/multi_copy_split.h"
910
#include "test_runner/test_runner.h"
@@ -57,6 +58,9 @@ void DBTest::runTest(std::vector<TestStatement>& statements, uint64_t checkpoint
5758
for (const auto& connName : connNames) {
5859
concurrentTests.try_emplace(connName, connectionsPaused, *connMap[connName], databasePath);
5960
}
61+
62+
bool skipFsmLeakCheck = false;
63+
6064
for (auto& statement : statements) {
6165
// special for testing import and export test cases
6266
if (statement.removeFileFlag) {
@@ -133,6 +137,10 @@ void DBTest::runTest(std::vector<TestStatement>& statements, uint64_t checkpoint
133137
split.run();
134138
continue;
135139
}
140+
if (statement.skipFSMLeakCheckerFlag) {
141+
skipFsmLeakCheck = true;
142+
continue;
143+
}
136144
if (statement.type == TestStatementType::LOG) {
137145
spdlog::info("DEBUG LOG: {}", statement.logMessage);
138146
continue;
@@ -153,6 +161,20 @@ void DBTest::runTest(std::vector<TestStatement>& statements, uint64_t checkpoint
153161
}
154162
}
155163
}
164+
165+
// Run FSM checker for all tests.
166+
if (!inMemMode && !skipFsmLeakCheck) {
167+
main::Connection* leakConn = nullptr;
168+
if (connNames.size() == 1) {
169+
// conn (originally null) is populated in the case that the connection in connMap is
170+
// reset. (ex. during import and export). Use conn in this case.
171+
leakConn = conn ? conn.get() : connMap.begin()->second.get();
172+
if (!leakConn) {
173+
throw TestException("FSM leak check has no available connection.");
174+
}
175+
FSMLeakChecker::checkForLeakedPages(leakConn);
176+
}
177+
}
156178
}
157179

158180
void ConcurrentTestExecutor::runStatements() const {
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#pragma once
2+
3+
#include <cstdint>
4+
5+
namespace kuzu {
6+
namespace main {
7+
class Connection;
8+
} // namespace main
9+
10+
namespace testing {
11+
12+
struct FSMLeakChecker {
13+
// Performs the whole leak check sequence; throws/asserts on failure
14+
static void checkForLeakedPages(main::Connection* conn);
15+
};
16+
17+
} // namespace testing
18+
} // namespace kuzu

test/include/test_runner/test_group.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ struct TestStatement {
8686
std::string newOutput;
8787
bool isPartOfStatementBlock = false;
8888

89+
// Execute by default
90+
bool skipFSMLeakCheckerFlag = false;
91+
8992
bool isValid() const { return type != TestStatementType::INVALID; }
9093
};
9194

test/include/test_runner/test_parser.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
namespace kuzu {
1212
namespace testing {
13-
1413
enum class TokenType {
1514
// header only
1615
DATASET,
@@ -49,6 +48,7 @@ enum class TokenType {
4948
MULTI_COPY_RANDOM,
5049
LOAD_DYNAMIC_EXTENSION,
5150
SEED,
51+
SKIP_FSM_LEAK_CHECKER,
5252

5353
CHECKPOINT_WAIT_TIMEOUT,
5454
CREATE_CONNECTION,
@@ -93,7 +93,7 @@ const std::unordered_map<std::string, TokenType> TOKEN_MAP = {{"-DATASET", Token
9393
{"-CREATE_DATASET_SCHEMA", TokenType::CREATE_DATASET_SCHEMA},
9494
{"-INSERT_DATASET_BY_ROW", TokenType::INSERT_DATASET_BY_ROW},
9595
{"-MULTI_COPY_RANDOM", TokenType::MULTI_COPY_RANDOM}, {"-LOOP", TokenType::LOOP},
96-
{"-ENDLOOP", TokenType::ENDLOOP}};
96+
{"-ENDLOOP", TokenType::ENDLOOP}, {"-SKIP_FSM_LEAK_CHECK", TokenType::SKIP_FSM_LEAK_CHECKER}};
9797

9898
class LogicToken {
9999
public:

test/test_files/dml_rel/create/create_small.test

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,7 @@ Runtime exception: Node(nodeOffset: 0) has more than one neighbour in table Live
298298
4|0|102
299299

300300
-CASE ScanUnCommittedRel
301+
-SKIP_FSM_LEAK_CHECK
301302
-TEST_FWD_ONLY_REL
302303
-STATEMENT create node table person (ID INt64, fName StRING, gender INT64, isStudent BoOLEAN, isWorker BOOLEAN, age INT64, eyeSight DOUBLE, birthdate DATE, registerTime TIMESTAMP, lastJobDuration interval, workedHours INT64[], usedNames STRING[], courseScoresPerTerm INT64[][], grades INT64[4], height float, u UUID, PRIMARY KEY (ID));
303304
---- ok

test/test_files/transaction/basic.test

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ Connection already has an active transaction. Cannot start a transaction within
3131
---- 0
3232

3333
-CASE RollbackWhenCloseDB
34+
-SKIP_FSM_LEAK_CHECK
3435
-STATEMENT CREATE NODE TABLE person(id string, primary key(id));
3536
---- ok
3637
-STATEMENT BEGIN TRANSACTION;

test/test_files/transaction/ddl/create_macro.test

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ result|6|5.600000|00:20:00|[1,3,2]
159159
9
160160

161161
-CASE createMacroReadTrxError
162+
-SKIP_FSM_LEAK_CHECK
162163
-STATEMENT BEGIN TRANSACTION READ ONLY
163164
---- ok
164165
-STATEMENT CREATE MACRO var_macro(x) AS x

test/test_files/transaction/local_storage.test

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
]
6969

7070
-CASE LocalTableScan
71+
-SKIP_FSM_LEAK_CHECK
7172
-STATEMENT BEGIN TRANSACTION;
7273
---- ok
7374
-INSERT_STATEMENT_BLOCK POPULATE_DATASET_TINYSNB
@@ -99,6 +100,7 @@
99100
7|9|1905-12-12|2020-03-01 12:11:41.6552|00:47:58|[peweeeeeeeeeeeeeeeee,kowje9w0eweeeeeeeee]|{locations: ['waterloo'], transfer: {day: 2000-01-01, amount: [1000,5000]}}|10|
100101

101102
-CASE LocalTableDetachDelele
103+
-SKIP_FSM_LEAK_CHECK
102104
-STATEMENT BEGIN TRANSACTION;
103105
---- ok
104106
-INSERT_STATEMENT_BLOCK POPULATE_DATASET_TINYSNB

0 commit comments

Comments
 (0)