From e5e0553cda6bf07cf3d3ff5723738cb6bf66965c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Szab=C3=B3?= Date: Tue, 1 Oct 2024 12:32:43 +0200 Subject: [PATCH 1/5] Disable client identity propagation in the OSS build --- mcrouter/ServerOnRequest.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/mcrouter/ServerOnRequest.h b/mcrouter/ServerOnRequest.h index 35738e675..045b985b3 100644 --- a/mcrouter/ServerOnRequest.h +++ b/mcrouter/ServerOnRequest.h @@ -11,7 +11,10 @@ #include #include +#ifndef MCROUTER_OSS_BUILD #include "core_infra_security/thrift_authentication_module/ClientIdentifierHelper.h" +#endif + #include "mcrouter/CarbonRouterClient.h" #include "mcrouter/RequestAclChecker.h" #include "mcrouter/config.h" @@ -165,7 +168,8 @@ class ServerOnRequest { auto& reqRef = rctx->req; auto& ctxRef = rctx->ctx; - // Set hashed TLS client identities on request to propogate from proxy -> +#ifndef MCROUTER_OSS_BUILD + // Set hashed TLS client identities on request to propagate from proxy -> // memcache server only IF enableKeyClientBinding_ is enabled. if (FOLLY_UNLIKELY(enableKeyClientBinding_) && ctxRef.getThriftRequestContext()) { @@ -180,6 +184,8 @@ class ServerOnRequest { std::get(mayBeHashedIdentities.value())); } } +#endif + // if we are reusing the request buffer, adjust the start offset and set // it to the request. if (reusableRequestBuffer) { From 447b0f6934de965b939a79d1da9d855651c71917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Szab=C3=B3?= Date: Thu, 14 Nov 2024 11:27:42 +0100 Subject: [PATCH 2/5] Add hostUniqueKey() SR shim --- mcrouter/mcrouter_sr_deps-impl.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mcrouter/mcrouter_sr_deps-impl.h b/mcrouter/mcrouter_sr_deps-impl.h index 97468f6a7..21e6e7808 100644 --- a/mcrouter/mcrouter_sr_deps-impl.h +++ b/mcrouter/mcrouter_sr_deps-impl.h @@ -28,6 +28,9 @@ class HostInfoLocation { const uint16_t* getTWTaskID() const { return nullptr; } + uint64_t hostUniqueKey() const noexcept { + return 0; + } private: const std::string ip_ = "127.0.0.1"; From 257c76a13603e517c1313d505bc6df0fbb741cf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Szab=C3=B3?= Date: Thu, 5 Dec 2024 14:20:51 +0100 Subject: [PATCH 3/5] Use FB_LOG_EVERY_MS in OSS --- mcrouter/CarbonRouterInstance-inl.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mcrouter/CarbonRouterInstance-inl.h b/mcrouter/CarbonRouterInstance-inl.h index ccbd4f30b..32773a81c 100644 --- a/mcrouter/CarbonRouterInstance-inl.h +++ b/mcrouter/CarbonRouterInstance-inl.h @@ -18,7 +18,9 @@ #include #include +#ifndef MCROUTER_OSS_BUILD #include +#endif #include "mcrouter/AsyncWriter.h" #include "mcrouter/CarbonRouterInstanceBase.h" @@ -131,7 +133,11 @@ CarbonRouterInstance* CarbonRouterInstance::createRaw( // Deleter is only called if unique_ptr::get() returns non-null. auto deleter = [](CarbonRouterInstance* inst) { +#ifdef MCROUTER_OSS_BUILD + FB_LOG_EVERY_MS(WARNING, 10000) << "Destroying CarbonRouterInstance"; +#else LOG_EVERY_MS(WARNING, 10000) << "Destroying CarbonRouterInstance"; +#endif delete inst; }; auto router = From 245850a2660d317e9a45b42295897da869ad4ed5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Szab=C3=B3?= Date: Wed, 11 Dec 2024 13:17:42 +0100 Subject: [PATCH 4/5] Introduce and use Ubuntu 24.04 build files --- .github/workflows/build.yml | 8 ++-- mcrouter/configure.ac | 6 +-- mcrouter/scripts/Makefile_ubuntu-24.04 | 41 ++++++++++++++++++ mcrouter/scripts/install_ubuntu_24.04.sh | 54 ++++++++++++++++++++++++ mcrouter/scripts/recipes/fbthrift.sh | 4 +- mcrouter/scripts/recipes/fizz.sh | 4 +- mcrouter/scripts/recipes/fmtlib.sh | 7 ++- mcrouter/scripts/recipes/folly.sh | 32 +------------- mcrouter/scripts/recipes/mvfst.sh | 4 +- mcrouter/scripts/recipes/wangle.sh | 4 +- 10 files changed, 115 insertions(+), 49 deletions(-) create mode 100644 mcrouter/scripts/Makefile_ubuntu-24.04 create mode 100755 mcrouter/scripts/install_ubuntu_24.04.sh diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0b3fcafd2..837c51605 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -11,14 +11,14 @@ on: jobs: build: - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 steps: - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Build dependencies run: | - ./mcrouter/scripts/install_ubuntu_20.04.sh "$(pwd)"/mcrouter-install deps + ./mcrouter/scripts/install_ubuntu_24.04.sh "$(pwd)"/mcrouter-install deps - name: Build mcrouter run: | mkdir -p "$(pwd)"/mcrouter-install/install - ./mcrouter/scripts/install_ubuntu_20.04.sh "$(pwd)"/mcrouter-install mcrouter + ./mcrouter/scripts/install_ubuntu_24.04.sh "$(pwd)"/mcrouter-install mcrouter diff --git a/mcrouter/configure.ac b/mcrouter/configure.ac index 8d64cb75b..a84c8b4a0 100644 --- a/mcrouter/configure.ac +++ b/mcrouter/configure.ac @@ -57,9 +57,9 @@ LT_INIT CXXFLAGS="-fno-strict-aliasing -std=c++20 $CXXFLAGS" CXXFLAGS="-W -Wall -Wextra -Wno-unused-parameter $CXXFLAGS" CXXFLAGS=" -Wno-missing-field-initializers -Wno-deprecated-declarations $CXXFLAGS" -CXXFLAGS="-DLIBMC_FBTRACE_DISABLE -DDISABLE_COMPRESSION $CXXFLAGS" +CXXFLAGS="-DLIBMC_FBTRACE_DISABLE -DDISABLE_COMPRESSION -DMCROUTER_OSS_BUILD $CXXFLAGS" -CFLAGS="-DLIBMC_FBTRACE_DISABLE -DDISABLE_COMPRESSION $CFLAGS" +CFLAGS="-DLIBMC_FBTRACE_DISABLE -DDISABLE_COMPRESSION -DMCROUTER_OSS_BUILD $CFLAGS" # Checks for glog and gflags # There are no symbols with C linkage, so we do a try-run @@ -199,7 +199,7 @@ AC_CHECK_FUNCS([gettimeofday \ LIBS="$LIBS $BOOST_LDFLAGS $BOOST_CONTEXT_LIB $BOOST_FILESYSTEM_LIB \ $BOOST_PROGRAM_OPTIONS_LIB $BOOST_SYSTEM_LIB $BOOST_REGEX_LIB \ $BOOST_THREAD_LIB -lpthread -pthread -ldl -lunwind \ - -lbz2 -llz4 -llzma -lsnappy -lzstd -latomic" + -lbz2 -llz4 -llzma -lsnappy -lzstd -latomic -lxxhash" AM_PATH_PYTHON([2.6],, [:]) AM_CONDITIONAL([HAVE_PYTHON], [test "$PYTHON" != :]) diff --git a/mcrouter/scripts/Makefile_ubuntu-24.04 b/mcrouter/scripts/Makefile_ubuntu-24.04 new file mode 100644 index 000000000..3dfe67b7f --- /dev/null +++ b/mcrouter/scripts/Makefile_ubuntu-24.04 @@ -0,0 +1,41 @@ +# Copyright (c) Facebook, Inc. and its affiliates. +# +# This source code is licensed under the MIT license found in the +# LICENSE file in the root directory of this source tree. + +RECIPES_DIR := ./recipes + +all: .folly-done .fizz-done .wangle-done .fmt-done .mvfst-done .fbthrift-done .gtest-done + ${RECIPES_DIR}/mcrouter.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR) + touch $@ + +mcrouter: + ${RECIPES_DIR}/mcrouter.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR) + touch $@ + +deps: .folly-done .fizz-done .wangle-done .fmt-done .mvfst-done .fbthrift-done + touch $@ + +.folly-done: .fmt-done + ${RECIPES_DIR}/folly.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR) + touch $@ + +.fizz-done: .folly-done + ${RECIPES_DIR}/fizz.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR) + touch $@ + +.wangle-done: .folly-done .fizz-done + ${RECIPES_DIR}/wangle.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR) + touch $@ + +.fmt-done: + ${RECIPES_DIR}/fmtlib.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR) + touch $@ + +.mvfst-done: .wangle-done + ${RECIPES_DIR}/mvfst.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR) + touch $@ + +.fbthrift-done: .folly-done .fizz-done .wangle-done .fmt-done .mvfst-done + ${RECIPES_DIR}/fbthrift.sh $(PKG_DIR) $(INSTALL_DIR) $(INSTALL_AUX_DIR) + touch $@ diff --git a/mcrouter/scripts/install_ubuntu_24.04.sh b/mcrouter/scripts/install_ubuntu_24.04.sh new file mode 100755 index 000000000..fe29a0e56 --- /dev/null +++ b/mcrouter/scripts/install_ubuntu_24.04.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env bash +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This source code is licensed under the MIT license found in the +# LICENSE file in the root directory of this source tree. + +set -ex + +BASE_DIR="$1" +TARGET="${2:-all}" + +[ -n "$BASE_DIR" ] || ( echo "Base dir missing"; exit 1 ) + +sudo apt-get update + +sudo apt-get install -y \ + autoconf \ + binutils-dev \ + bison \ + cmake \ + flex \ + g++ \ + gcc \ + git \ + libboost1.83-all-dev \ + libbz2-dev \ + libdouble-conversion-dev \ + libevent-dev \ + libfast-float-dev \ + libgflags-dev \ + libgoogle-glog-dev \ + libgmock-dev \ + libgtest-dev \ + libjemalloc-dev \ + liblz4-dev \ + liblzma-dev \ + liblzma5 \ + libsnappy-dev \ + libsodium-dev \ + libssl-dev \ + libtool \ + libunwind8-dev \ + libxxhash-dev \ + libzstd-dev \ + make \ + ninja-build \ + pkg-config \ + python3-dev \ + ragel \ + sudo + +cd "$(dirname "$0")" || ( echo "cd fail"; exit 1 ) + +./get_and_build_by_make.sh "Makefile_ubuntu-24.04" "$TARGET" "$BASE_DIR" diff --git a/mcrouter/scripts/recipes/fbthrift.sh b/mcrouter/scripts/recipes/fbthrift.sh index a4e84bef8..eecb1a05f 100755 --- a/mcrouter/scripts/recipes/fbthrift.sh +++ b/mcrouter/scripts/recipes/fbthrift.sh @@ -21,5 +21,5 @@ fi cd "$PKG_DIR/fbthrift/build" || die "cd fbthrift failed" CXXFLAGS="$CXXFLAGS -fPIC" \ -cmake .. -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -make $MAKE_ARGS && make install $MAKE_ARGS +cmake .. -G Ninja -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" +cmake --build . && cmake --install . diff --git a/mcrouter/scripts/recipes/fizz.sh b/mcrouter/scripts/recipes/fizz.sh index d97c4c208..b5495fcd1 100755 --- a/mcrouter/scripts/recipes/fizz.sh +++ b/mcrouter/scripts/recipes/fizz.sh @@ -20,5 +20,5 @@ fi cd "$PKG_DIR/fizz/fizz/" || die "cd fail" -cmake . -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DBUILD_TESTS=OFF -make $MAKE_ARGS && make install $MAKE_ARGS +cmake . -G Ninja -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DBUILD_TESTS=OFF +cmake --build . && cmake --install . diff --git a/mcrouter/scripts/recipes/fmtlib.sh b/mcrouter/scripts/recipes/fmtlib.sh index 0bd074dea..63b63acdd 100755 --- a/mcrouter/scripts/recipes/fmtlib.sh +++ b/mcrouter/scripts/recipes/fmtlib.sh @@ -7,7 +7,7 @@ source common.sh if [[ ! -d "$PKG_DIR/fmt" ]]; then - git clone https://github.com/fmtlib/fmt + git clone --depth 1 -b 11.0.2 https://github.com/fmtlib/fmt.git cd "$PKG_DIR/fmt" || die "cd failed" mkdir "$PKG_DIR/fmt/build" fi @@ -15,6 +15,5 @@ fi cd "$PKG_DIR/fmt/build" || die "cd fmt failed" CXXFLAGS="$CXXFLAGS -fPIC" \ - cmake .. -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -make $MAKE_ARGS && make install $MAKE_ARGS - + cmake .. -G Ninja -DFMT_TEST=Off -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" +cmake --build . && cmake --install . diff --git a/mcrouter/scripts/recipes/folly.sh b/mcrouter/scripts/recipes/folly.sh index 4ff8f9f1a..b351677b0 100755 --- a/mcrouter/scripts/recipes/folly.sh +++ b/mcrouter/scripts/recipes/folly.sh @@ -18,42 +18,14 @@ if [[ ! -d folly ]]; then fi fi -if [ ! -d /usr/include/double-conversion ]; then - if [ ! -d "$PKG_DIR/double-conversion" ]; then - cd "$PKG_DIR" || die "cd fail" - git clone https://github.com/google/double-conversion.git - fi - cd "$PKG_DIR/double-conversion" || die "cd fail" - - # Workaround double-conversion CMakeLists.txt changes that - # are incompatible with cmake-2.8 - git checkout ea970f69edacf66bd3cba2892be284b76e9599b0 - cmake . -DBUILD_SHARED_LIBS=ON -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" - make $MAKE_ARGS && make install $MAKE_ARGS - - export LDFLAGS="-L$INSTALL_DIR/lib -ldl $LDFLAGS" - export CPPFLAGS="-I$INSTALL_DIR/include $CPPFLAGS" -fi - -if [ ! -d "$PKG_DIR/zstd" ]; then - cd "$PKG_DIR" || die "cd fail" - git clone https://github.com/facebook/zstd - - cd "$PKG_DIR/zstd" || die "cd fail" - - # Checkout zstd-1.4.9 release - git checkout e4558ffd1dc49399faf4ee5d85abed4386b4dcf5 - cmake -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" build/cmake/ - make $MAKE_ARGS && make install $MAKE_ARGS -fi - cd "$PKG_DIR/folly/folly/" || die "cd fail" CXXFLAGS="$CXXFLAGS -fPIC" \ LD_LIBRARY_PATH="$INSTALL_DIR/lib:$LD_LIBRARY_PATH" \ LD_RUN_PATH="$INSTALL_DIR/lib:$LD_RUN_PATH" \ cmake .. \ + -G Ninja \ -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" \ -DCMAKE_INCLUDE_PATH="$INSTALL_DIR/lib" \ -DCMAKE_LIBRARY_PATH="$INSTALL_DIR/lib" -make $MAKE_ARGS && make install $MAKE_ARGS +cmake --build . && cmake --install . diff --git a/mcrouter/scripts/recipes/mvfst.sh b/mcrouter/scripts/recipes/mvfst.sh index 73c69a1c3..aa7c02327 100755 --- a/mcrouter/scripts/recipes/mvfst.sh +++ b/mcrouter/scripts/recipes/mvfst.sh @@ -11,6 +11,6 @@ if [ ! -d "$PKG_DIR/mvfst" ]; then cd "$PKG_DIR/mvfst" || die "cd fail" cmake . \ - -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DBUILD_TESTS=OFF - make $MAKE_ARGS && make install $MAKE_ARGS + -G Ninja -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DBUILD_TESTS=OFF + cmake --build . && cmake --install . fi diff --git a/mcrouter/scripts/recipes/wangle.sh b/mcrouter/scripts/recipes/wangle.sh index 38e028e62..e700d73eb 100755 --- a/mcrouter/scripts/recipes/wangle.sh +++ b/mcrouter/scripts/recipes/wangle.sh @@ -20,5 +20,5 @@ fi cd "$PKG_DIR/wangle/wangle/" || die "cd fail" -cmake . -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DBUILD_TESTS=OFF -make $MAKE_ARGS && make install $MAKE_ARGS +cmake . -G Ninja -DCMAKE_INSTALL_PREFIX="$INSTALL_DIR" -DBUILD_TESTS=OFF +cmake --build . && cmake --install . From 9516a3f6795457df5a853642061c473f7ba8cbec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Szab=C3=B3?= Date: Mon, 16 Dec 2024 11:07:27 +0100 Subject: [PATCH 5/5] Remove redundant logging from CarbonRouterInstance deleter --- mcrouter/CarbonRouterInstance-inl.h | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/mcrouter/CarbonRouterInstance-inl.h b/mcrouter/CarbonRouterInstance-inl.h index 32773a81c..bfb385f05 100644 --- a/mcrouter/CarbonRouterInstance-inl.h +++ b/mcrouter/CarbonRouterInstance-inl.h @@ -18,10 +18,6 @@ #include #include -#ifndef MCROUTER_OSS_BUILD -#include -#endif - #include "mcrouter/AsyncWriter.h" #include "mcrouter/CarbonRouterInstanceBase.h" #include "mcrouter/ExecutorObserver.h" @@ -131,15 +127,8 @@ CarbonRouterInstance* CarbonRouterInstance::createRaw( initFailureLogger(); } - // Deleter is only called if unique_ptr::get() returns non-null. - auto deleter = [](CarbonRouterInstance* inst) { -#ifdef MCROUTER_OSS_BUILD - FB_LOG_EVERY_MS(WARNING, 10000) << "Destroying CarbonRouterInstance"; -#else - LOG_EVERY_MS(WARNING, 10000) << "Destroying CarbonRouterInstance"; -#endif - delete inst; - }; + // Custom deleter since ~CarbonRouterInstance() is private. + auto deleter = [](CarbonRouterInstance* inst) { delete inst; }; auto router = std::unique_ptr, decltype(deleter)>( new CarbonRouterInstance(std::move(input_options)),