Skip to content

Commit

Permalink
Fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
mszabo-wikia committed Jul 31, 2024
1 parent 8747563 commit 183ecd5
Show file tree
Hide file tree
Showing 17 changed files with 142 additions and 42 deletions.
62 changes: 53 additions & 9 deletions mcrouter/lib/network/test/AsyncMcClientTestSync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* LICENSE file in the root directory of this source tree.
*/

#include <fstream>
#include <stdexcept>
#include <string>

#include <gtest/gtest.h>
Expand Down Expand Up @@ -869,6 +871,9 @@ TEST(AsyncMcClient, caretVersionUserSpecified) {
}

TEST(AsyncMcClient, caretAdditionalFields) {
#ifdef LIBMC_FBTRACE_DISABLE
GTEST_SKIP() << "Tracing is disabled in the OSS build";
#endif
TestServer::Config config;
config.useSsl = false;
auto server = TestServer::create(std::move(config));
Expand Down Expand Up @@ -958,6 +963,26 @@ using TFOTestParams = std::tuple<bool, bool, bool, SecurityMech>;

class AsyncMcClientTFOTest : public TestWithParam<TFOTestParams> {};

/**
* Check whether the current host system supports TCP fastopen.
*/
bool tfoSupportedOnHost() {
try {
constexpr int kClientSupport = 1;
constexpr int kDataInOpeningSynWithoutCookie = 4;
int flags;
std::ifstream procFs("/proc/sys/net/ipv4/tcp_fastopen");
procFs >> flags;

return
(flags & kClientSupport) == kClientSupport &&
(flags & kDataInOpeningSynWithoutCookie) == kDataInOpeningSynWithoutCookie;
} catch (std::exception& e) {
LOG(ERROR) << "Could not read TCP fastopen sysctl: " << e.what();
return false;
}
}

TEST_P(AsyncMcClientTFOTest, testTfoWithSSL) {
auto serverEnabled = std::get<0>(GetParam());
auto clientEnabled = std::get<1>(GetParam());
Expand All @@ -971,9 +996,10 @@ TEST_P(AsyncMcClientTFOTest, testTfoWithSSL) {

auto offloadHandshake = std::get<2>(GetParam());
auto constexpr nConnAttempts = 10;
auto tfoSupported = tfoSupportedOnHost();

auto mech = std::get<3>(GetParam());
auto sendReq = [serverEnabled, clientEnabled, mech](TestClient& client) {
auto sendReq = [serverEnabled, clientEnabled, tfoSupported, mech](TestClient& client) {
client.setConnectionStatusCallbacks(
[&](const folly::AsyncTransportWrapper& sock, int64_t) {
if (mech == SecurityMech::TLS_TO_PLAINTEXT) {
Expand All @@ -982,10 +1008,16 @@ TEST_P(AsyncMcClientTFOTest, testTfoWithSSL) {
auto stats = socket->getStats();
if (clientEnabled) {
EXPECT_TRUE(stats.tfoAttempted);
EXPECT_TRUE(stats.tfoFinished);
// we can not guarantee socket->getTFOSucceeded() will return true
// unless there are specific kernel + host settings applied
if (!serverEnabled) {

if (tfoSupported) {
EXPECT_TRUE(stats.tfoFinished);
if (serverEnabled) {
EXPECT_TRUE(stats.tfoSuccess);
} else {
EXPECT_FALSE(stats.tfoSuccess);
}
} else {
EXPECT_FALSE(stats.tfoFinished);
EXPECT_FALSE(stats.tfoSuccess);
}
} else {
Expand All @@ -995,10 +1027,16 @@ TEST_P(AsyncMcClientTFOTest, testTfoWithSSL) {
auto* socket = sock.getUnderlyingTransport<folly::AsyncSocket>();
if (clientEnabled) {
EXPECT_TRUE(socket->getTFOAttempted());
EXPECT_TRUE(socket->getTFOFinished());
// we can not guarantee socket->getTFOSucceeded() will return true
// unless there are specific kernel + host settings applied
if (!serverEnabled) {

if (tfoSupported) {
EXPECT_TRUE(socket->getTFOFinished());
if (serverEnabled) {
EXPECT_TRUE(socket->getTFOSucceded());
} else {
EXPECT_FALSE(socket->getTFOSucceded());
}
} else {
EXPECT_FALSE(socket->getTFOFinished());
EXPECT_FALSE(socket->getTFOSucceded());
}
} else {
Expand Down Expand Up @@ -1143,6 +1181,9 @@ TEST_P(AsyncMcClientSSLOffloadTest, closeNow) {
EXPECT_FALSE(upCalled);
EXPECT_TRUE(downReason.has_value());
EXPECT_EQ(*downReason, ConnectionDownReason::ABORTED);

server->shutdown();
server->join();
}

TEST_P(AsyncMcClientSSLOffloadTest, clientReset) {
Expand All @@ -1164,6 +1205,9 @@ TEST_P(AsyncMcClientSSLOffloadTest, clientReset) {
evb.loopOnce();
client.reset();
evb.loop();

server->shutdown();
server->join();
}

INSTANTIATE_TEST_CASE_P(AsyncMcClientTest, AsyncMcClientSSLOffloadTest, Bool());
1 change: 1 addition & 0 deletions mcrouter/lib/network/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ target_link_libraries(
FBThrift::thrift-core)

add_executable(mcrouter_mock_mc_thrift_server MockMc.cpp MockMcThriftServer.cpp)
set_property(TARGET mcrouter_mock_mc_thrift_server PROPERTY OUTPUT_NAME mock_mc_thrift_server)

target_link_libraries(
mcrouter_mock_mc_thrift_server
Expand Down
4 changes: 4 additions & 0 deletions mcrouter/lib/network/test/MockMc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,10 @@ MockMc::findUnexpired(folly::StringPiece key) {
return it;
}

size_t MockMc::itemCount() const noexcept {
return citems_.size();
}

void MockMc::flushAll() {
citems_.clear();
}
Expand Down
2 changes: 2 additions & 0 deletions mcrouter/lib/network/test/MockMc.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ class MockMc {

CasResult cas(folly::StringPiece key, Item item, uint64_t token);

size_t itemCount() const noexcept;

/**
* clear all items
*/
Expand Down
7 changes: 7 additions & 0 deletions mcrouter/lib/network/test/MockMcOnRequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ namespace facebook {
namespace memcache {

static std::atomic_uint64_t cmd_get_count{0};
static std::atomic_uint64_t cmd_lease_get_count{0};
static std::atomic_uint64_t cmd_lease_set_count{0};

class MockMcOnRequest {
public:
Expand Down Expand Up @@ -127,6 +129,7 @@ class MockMcOnRequest {
template <class Context>
void onRequest(Context&& ctx, McLeaseGetRequest&& req) {
using Reply = McLeaseGetReply;
++cmd_lease_get_count;

auto key = req.key_ref()->fullKey().str();

Expand All @@ -144,6 +147,7 @@ class MockMcOnRequest {
template <class Context>
void onRequest(Context&& ctx, McLeaseSetRequest&& req) {
using Reply = McLeaseSetReply;
++cmd_lease_set_count;

auto key = req.key_ref()->fullKey().str();

Expand Down Expand Up @@ -355,6 +359,9 @@ class MockMcOnRequest {
if (key == "__mockmc__") {
StatsReply stats;
stats.addStat("cmd_get_count", cmd_get_count.load());
stats.addStat("cmd_lease_get", cmd_lease_get_count.load());
stats.addStat("cmd_lease_set", cmd_lease_set_count.load());
stats.addStat("total_items", mc_.itemCount());
Context::reply(std::move(ctx), stats.getReply());
} else {
Context::reply(std::move(ctx), Reply(carbon::Result::BAD_COMMAND));
Expand Down
2 changes: 2 additions & 0 deletions mcrouter/lib/network/test/TestClientServerUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ void TestServerOnRequest::onRequest(
processReply(std::move(ctx), Reply(carbon::Result::NOTFOUND));
} else if (req.key_ref()->fullKey() == "shutdown") {
shutdownLock_.post();
// TODO
std::this_thread::sleep_for(std::chrono::milliseconds(100));
processReply(std::move(ctx), Reply(carbon::Result::NOTFOUND));
flushQueue();
} else if (req.key_ref()->fullKey() == "busy") {
Expand Down
14 changes: 10 additions & 4 deletions mcrouter/test/MCProcess.py
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,6 @@ def stats(self, spec=None):
if spec:
q = "stats {spec}\r\n".format(spec=spec)
self._sendall(q)

s = {}
line = None
fds = select.select([self.fd], [], [], 20.0)
Expand Down Expand Up @@ -800,13 +799,14 @@ def __init__(self, args, port=None, base_dir=None):
self.stats_dir,
"--debug-fifo-root",
self.debug_fifo_root,
"--rss-limit-mb",
"16384",
"--fibers-stack-size",
"65536",
]
)

if not McrouterGlobals.ossVersion():
args.extend(["--rss-limit-mb", "16384"])

listen_sock = None
pass_fds = []
if port is None:
Expand Down Expand Up @@ -1030,7 +1030,8 @@ def __init__(self, port=None, ssl_port=None, extra_args=None):
pass_fds = []

# if mockmc is used here, we initialize the same way as MockMemcached
if McrouterGlobals.binPath("mockmc") == args[0]:
self.is_mock_server = McrouterGlobals.binPath("mockmc") == args[0]
if self.is_mock_server:
if port is None:
listen_sock = create_listen_socket()
port = listen_sock.getsockname()[1]
Expand Down Expand Up @@ -1103,6 +1104,11 @@ def __init__(self, port=None, ssl_port=None, extra_args=None):
tries -= 1
self.disconnect()

def stats(self, spec=None):
if self.is_mock_server:
return super().stats('__mockmc__')
return super().stats(spec)

def getsslport(self):
return self.ssl_port

Expand Down
1 change: 1 addition & 0 deletions mcrouter/test/cpp_unit_tests/McrouterClientUsage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ TEST(CarbonRouterClient, basicUsageSameThreadClient) {
// gracefully. This ensures graceful destruction of the static
// CarbonRouterInstance instance.
router->shutdown();
ioThreadPool->join();
ioThreadPool.reset();
EXPECT_TRUE(replyReceived);
}
Expand Down
5 changes: 5 additions & 0 deletions mcrouter/test/mcrouter_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ def binPath(name):
"mcrouter": "./mcrouter/mcrouter",
"mcpiper": "./mcrouter/tools/mcpiper/mcpiper",
"mockmc": "./mcrouter/lib/network/test/mock_mc_server",
"mockmcthrift": "./mcrouter/lib/network/test/mock_mc_thrift_server",
"mockmcdual": "./mcrouter/lib/network/test/mock_mc_server_dual",
"prodmc": "./mcrouter/lib/network/test/mock_mc_server",
}
Expand All @@ -34,6 +35,10 @@ def preprocessArgs(args):
def useThriftClient():
return False

@staticmethod
def ossVersion() -> bool:
return True

@staticmethod
def createThriftTestClient(addr, port) -> ThriftTestClient:
raise NotImplementedError
6 changes: 4 additions & 2 deletions mcrouter/test/test_async_files_attr.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@
import os
import time

from libfb.py import parutil
from mcrouter.test.McrouterTestCase import McrouterTestCase

from mcrouter.test.config import McrouterGlobals

class TestAsyncFilesAttr(McrouterTestCase):
stat_prefix = "libmcrouter.mcrouter.0."
Expand Down Expand Up @@ -53,6 +52,9 @@ def test_stats_no_requests(self):
self.check_stats(mcrouter.stats_dir)

def test_async_files_attr(self):
if McrouterGlobals.ossVersion():
self.skipTest("The mcrouter client is not available in the OSS build")
from libfb.py import parutil
mcrouter = self.add_mcrouter(self.config, extra_args=self.extra_args)
binary = parutil.get_file_path("mcrouter/client_binary")
port = str(mcrouter.getport())
Expand Down
7 changes: 7 additions & 0 deletions mcrouter/test/test_axonlog.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# LICENSE file in the root directory of this source tree.

from mcrouter.test.McrouterTestCase import McrouterTestCase
from mcrouter.test.config import McrouterGlobals
from mcrouter.test.mock_servers import DeadServer


Expand All @@ -19,6 +20,9 @@ def setUp(self):
class TestAxonProxyFailedDelete(TestAxonLogBase):

def setUp(self):
if McrouterGlobals.ossVersion():
self.skipTest("Axon logging is not available in the OSS build")

self.mc1 = self.add_server(DeadServer(5000))
self.mr = self.add_mcrouter(self.config, extra_args=self.extra_args)

Expand All @@ -43,6 +47,9 @@ class TestAxonLogAllDelete(TestAxonLogBase):
config = "./mcrouter/test/test_axon_log_alldelete.json"

def setUp(self):
if McrouterGlobals.ossVersion():
self.skipTest("Axon logging is not available in the OSS build")

self.mr = self.add_mcrouter(self.config, extra_args=self.extra_args)

def test_all_delete(self):
Expand Down
8 changes: 7 additions & 1 deletion mcrouter/test/test_carbonlookaside_route.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ def __init__(
"TEMPLATE_PREFIX": prefix,
"TEMPLATE_TTL": ttl,
"TEMPLATE_TTL_UNIT_MS": has_ms_ttl,
"TEMPLATE_FILENAME": "file:" + self.tmpFlavorFile.name,
"TEMPLATE_FILENAME": self.tmpFlavorFile.name,
"TEMPLATE_LEASE_ENABLE": lease_enable,
"TEMPLATE_LEASE_WAIT_INTERVAL": lease_interval,
"TEMPLATE_LEASE_NUM_RETRIES": lease_num_retries,
Expand Down Expand Up @@ -159,6 +159,7 @@ def setUp(self):
self.mcrouter = self.add_mcrouter(self.config, extra_args=self.extra_args)

def tearDown(self):
super().tearDown()
self.tmpConfig.cleanup()

def test_carbonlookaside_basic(self):
Expand Down Expand Up @@ -220,6 +221,7 @@ def setUp(self):
self.mcrouter = self.add_mcrouter(self.config, extra_args=self.extra_args)

def tearDown(self):
super().tearDown()
self.tmpConfig.cleanup()

def test_carbonlookaside_ttl_expiry(self):
Expand Down Expand Up @@ -297,6 +299,7 @@ def setUp(self):
self.mcrouter = self.add_mcrouter(self.config, extra_args=self.extra_args)

def tearDown(self):
super().tearDown()
self.tmpConfig.cleanup()

def test_carbonlookaside_basic_leases(self):
Expand Down Expand Up @@ -360,6 +363,7 @@ def setUp(self):
self.mcrouter2 = self.add_mcrouter(self.config, extra_args=self.extra_args)

def tearDown(self):
super().tearDown()
self.tmpConfig.cleanup()

def async_get(self, key, ret):
Expand Down Expand Up @@ -425,6 +429,7 @@ def setUp(self):
self.mcrouter = self.add_mcrouter(self.config, extra_args=self.extra_args)

def tearDown(self):
super().tearDown()
self.tmpConfig.cleanup()

def test_carbonlookaside_ttl_ms_expiry_config(self):
Expand Down Expand Up @@ -465,6 +470,7 @@ def setUp(self):
self.mcrouter = self.add_mcrouter(self.config, extra_args=self.extra_args)

def tearDown(self):
super().tearDown()
self.tmpConfig.cleanup()

def test_carbonlookaside_ttl_ms_expiry(self):
Expand Down
7 changes: 5 additions & 2 deletions mcrouter/test/test_mcrouter_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import time
from threading import Thread

from mcrouter.test.MCProcess import Mcrouter, McrouterClient, Memcached
from mcrouter.test.MCProcess import Mcrouter, McrouterClient, Memcached, BaseDirectory
from mcrouter.test.McrouterTestCase import McrouterTestCase


Expand Down Expand Up @@ -1137,7 +1137,10 @@ class TestMcrouterWithRetries(McrouterTestCase):
invalid_config_with_retries = (
"./mcrouter/test/test_basic_l1_l2_sizesplit_retry_invalid.json"
)
extra_args = ["--validate-config=run"]

def setUp(self):
self.config_dump_root = BaseDirectory("config_dump")
self.extra_args = ["--validate-config=run", "--config-dump-root", self.config_dump_root.path]

def test_valid_retries(self):
mcr = self.add_mcrouter(
Expand Down
Loading

0 comments on commit 183ecd5

Please sign in to comment.