Skip to content

Commit

Permalink
Omnibus CDB improvements (#194)
Browse files Browse the repository at this point in the history
* Omnibus patch for CDB

* clean up a bunch of lint errors
* move to using failure crate
* cdb-generation-utility for running performance tests
* selectable mmap or heap backends
* bindgen the cdb_rs.h include file so we can share structures between C and Rust
* merged cdb_ffi and cdb_rs

* build tweak to integrate rust builds

* make rust CI optional

* kevyang feedback, add test for cdb_destroy pointer nulling

* fix clippy lints

* more kyang feedback

* remove twitter-specific stuff, address yao's feedback

* add distinguishing marker to Cargo.toml in cdb dir

* ignore dist dir

* fix scripts to be compatible with ancient version of git in centos
  • Loading branch information
slyphon authored and Yao Yue committed Jul 30, 2018
1 parent 47edd8f commit 58bf871
Show file tree
Hide file tree
Showing 40 changed files with 924 additions and 436 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,4 @@ compile_commands.json

*.cmd
core
CMAKE_BINARY_DIR
13 changes: 12 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,18 @@ matrix:
- gcc-4.9
- libsubunit-dev

# gcc 5 on linux
# gcc 5 on linux *without rust*
#
- env:
- C_COMPILER=gcc-5
addons:
apt:
<<: *apt
packages:
- gcc-5
- libsubunit-dev

# gcc 5 on linux *with* rust
- env:
- C_COMPILER=gcc-5
- RUST_ENABLED=1
Expand Down
4 changes: 3 additions & 1 deletion ci/before-install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ TEMP="$(mktemp -d -t TEMP.XXXXXXX)" || die "failed to make tmpdir"
cleanup() { [[ -n "${TEMP:-}" ]] && rm -rf "${TEMP}"; }
trap cleanup EXIT

TOPLEVEL="$(git -C "$(cd "$(dirname "$0")" >/dev/null || exit 1; pwd)" rev-parse --show-toplevel)" || die 'failed to find TOPLEVEL'
realpath() { python -c "import os,sys; print os.path.realpath(sys.argv[1])" "$1"; }

TOPLEVEL="$(cd "$(dirname "$(realpath "$0" >/dev/null || exit 1)")" && git rev-parse --show-toplevel)" || die 'failed to find TOPLEVEL'

# for osx: 0. update brew; 1. install cmake if missing; 2. (gcc) unlink pre-installed gcc; 3. (gcc) install desired version of gcc

Expand Down
4 changes: 3 additions & 1 deletion ci/install-check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ TEMP="$(mktemp -d -t TEMP.XXXXXXX)" || die "failed to make tmpdir"
cleanup() { [[ -n "${TEMP:-}" ]] && rm -rf "${TEMP}"; }
trap cleanup EXIT

TOPLEVEL="$(git -C "$(cd "$(dirname "$0")" >/dev/null || exit 1; pwd)" rev-parse --show-toplevel)" || die 'failed to find TOPLEVEL'
realpath() { python -c "import os,sys; print os.path.realpath(sys.argv[1])" "$1"; }

TOPLEVEL="$(cd "$(dirname "$(realpath "$0" >/dev/null || exit 1)")" && git rev-parse --show-toplevel)" || die 'failed to find TOPLEVEL'

CHECK_VERSION=0.12.0
CHECK_TARBALL="check-${CHECK_VERSION}.tar.gz"
Expand Down
20 changes: 20 additions & 0 deletions ci/local-clean-build-and-test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/bin/bash

set -euo pipefail
IFS=$'\n\t'

die() { echo "fatal: $*" >&2; exit 1; }

TEMP="$(mktemp -d -t TEMP.XXXXXXX)" || die "failed to make tmpdir"
cleanup() { [[ -n "${TEMP:-}" ]] && rm -rf "${TEMP}"; }
trap cleanup EXIT


if [[ -n "$(git status --porcelain)" ]]; then
die "dirty working copy state, please commit changes before running this script"
fi

git archive --format=tar --prefix=pelikan/ HEAD .|tar -C "$TEMP" -xf-
cd "$TEMP/pelikan"

bash ./ci/run.sh
19 changes: 7 additions & 12 deletions ci/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ export PATH=$HOME/.cargo/bin:$PATH

CMAKE_ARGS=(
-DBUILD_AND_INSTALL_CHECK=yes
-DTARGET_CDB=yes
-DHAVE_RUST=yes
-DRUST_VERBOSE_BUILD=1
)

if [[ -n "${RUST_ENABLED:-}" ]]; then
CMAKE_ARGS+=( -DTARGET_CDB=yes -DHAVE_RUST=yes -DRUST_VERBOSE_BUILD=1 )
fi

# TODO: run cmake3 on centos hosts

mkdir -p _build && ( cd _build && cmake ${CMAKE_ARGS[@]} .. && make -j && make check ) || die 'make failed'
Expand All @@ -30,18 +31,12 @@ egrep -r ":F:|:E:" . |grep -v 'Binary file' || true

set +e

( cd src/storage/cdb && env RUST_BACKTRACE=full cargo test )
if [[ -n "${RUST_ENABLED:-}" ]]; then
( cd src/storage/cdb && env RUST_BACKTRACE=full cargo test )
fi

RESULT=$?

if [[ "$(uname -s)" == "Darwin" ]]; then
if [[ $RESULT -ne 0 ]]; then
echo "Rust test failed on OSX, but this does not fail the build" >&2
fi

exit 0
fi

if [[ $RESULT -ne 0 ]]; then
echo "Build failure" >&2
exit $RESULT
Expand Down
3 changes: 2 additions & 1 deletion cmake/CMakeCargo.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ function(cargo_build)
cmake_parse_arguments(CARGO "" "NAME" "" ${ARGN})
string(REPLACE "-" "_" LIB_NAME ${CARGO_NAME})

get_filename_component(CARGO_TARGET_DIR "${CMAKE_CURRENT_BINARY_DIR}" ABSOLUTE)
get_filename_component(CARGO_TARGET_DIR "${CMAKE_BINARY_DIR}/target" ABSOLUTE)
message(STATUS "CARGO_TARGET_DIR ${CARGO_TARGET_DIR}")

cargo_set_lib_target(LIB_NAME)

Expand Down
5 changes: 4 additions & 1 deletion deps/ccommon/ci/before-install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ TEMP="$(mktemp -d -t TEMP.XXXXXXX)" || die "failed to make tmpdir"
cleanup() { [[ -n "${TEMP:-}" ]] && rm -rf "${TEMP}"; }
trap cleanup EXIT

TOPLEVEL="$(git -C "$(cd "$(dirname "$0")" >/dev/null || exit 1; pwd)" rev-parse --show-toplevel)" || die 'failed to find TOPLEVEL'
realpath() { python -c "import os,sys; print os.path.realpath(sys.argv[1])" "$1"; }

TOPLEVEL="$(cd "$(dirname "$(realpath "$0" >/dev/null || exit 1)")" && git rev-parse --show-toplevel)" || die 'failed to find TOPLEVEL'


# for osx: 0. update brew; 1. install cmake if missing; 2. (gcc) unlink pre-installed gcc; 3. (gcc) install desired version of gcc

Expand Down
5 changes: 4 additions & 1 deletion deps/ccommon/ci/install-check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ TEMP="$(mktemp -d -t TEMP.XXXXXXX)" || die "failed to make tmpdir"
cleanup() { [[ -n "${TEMP:-}" ]] && rm -rf "${TEMP}"; }
trap cleanup EXIT

TOPLEVEL="$(git -C "$(cd "$(dirname "$0")" >/dev/null || exit 1; pwd)" rev-parse --show-toplevel)" || die 'failed to find TOPLEVEL'
realpath() { python -c "import os,sys; print os.path.realpath(sys.argv[1])" "$1"; }

TOPLEVEL="$(cd "$(dirname "$(realpath "$0" >/dev/null || exit 1)")" && git rev-parse --show-toplevel)" || die 'failed to find TOPLEVEL'


CHECK_VERSION=0.12.0
CHECK_TARBALL="check-${CHECK_VERSION}.tar.gz"
Expand Down
2 changes: 1 addition & 1 deletion deps/ccommon/rust/ccommon_rs/src/bstring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ impl BStr {

// it's ok to ignore the cast_ptr_alignment lint because we're going
// *mut CCbstring -> &BStr -> *mut CCbstring
#[allow(cast_ptr_alignment)]
#[allow(unknown_lints)]
#[allow(cast_ptr_alignment)]
#[inline]
pub fn as_ptr(&self) -> *mut CCbstring {
(&*self) as *const _ as *mut _
Expand Down
4 changes: 2 additions & 2 deletions src/server/cdb/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ set(SOURCE
stats.c)

set(MODULES
cdb_ffi_static
cdb_rs_static
core
protocol_admin
protocol_memcache
Expand All @@ -25,7 +25,7 @@ set(TARGET_NAME ${PROJECT_NAME}_cdb)

add_executable(${TARGET_NAME} ${SOURCE})
target_link_libraries(${TARGET_NAME} ${MODULES} ${LIBS})
add_dependencies(${TARGET_NAME} cdb_ffi_static)
add_dependencies(${TARGET_NAME} cdb_rs_static)

install(TARGETS ${TARGET_NAME} RUNTIME DESTINATION bin)
add_dependencies(service ${TARGET_NAME})
16 changes: 6 additions & 10 deletions src/server/cdb/data/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#include "process.h"

#include "protocol/data/memcache_include.h"
#include "storage/cdb/cdb.h"
#include "storage/cdb/cdb_rs.h"

#include <cc_array.h>
#include <cc_debug.h>
Expand All @@ -26,10 +26,10 @@ static struct bstring value_buf;
static bool process_init = false;
static process_metrics_st *process_metrics = NULL;

static struct CDBHandle *cdb_handle = NULL;
static struct cdb_handle *cdb_handle = NULL;

void
process_setup(process_options_st *options, process_metrics_st *metrics, struct CDBHandle *handle)
process_setup(process_options_st *options, process_metrics_st *metrics, struct cdb_handle *handle)
{
log_info("set up the %s module", CDB_PROCESS_MODULE_NAME);

Expand Down Expand Up @@ -68,15 +68,12 @@ process_teardown(void)
}

if (cdb_handle != NULL) {
struct CDBHandle *p = cdb_handle;
cdb_handle = NULL;
cdb_handle_destroy(p);
cdb_handle_destroy(&cdb_handle);
}

if (value_buf.data != NULL) {
char *p = value_buf.data;
value_buf.data = NULL;
value_buf.len = 0;
bstring_init(&value_buf);
cc_free(p);
}

Expand All @@ -101,8 +98,7 @@ _get_key(struct response *rsp, struct bstring *key)
rsp->key = *key;
rsp->flag = 0;
rsp->vcas = 0;
rsp->vstr.len = vstr->len;
rsp->vstr.data = vstr->data;
rsp->vstr = *vstr;

log_verb("found key at %p, location %p", key, vstr);
return true;
Expand Down
4 changes: 2 additions & 2 deletions src/server/cdb/data/process.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once

#include "storage/cdb/cdb.h"
#include "storage/cdb/cdb_rs.h"

#include <buffer/cc_buf.h>
#include <cc_metric.h>
Expand Down Expand Up @@ -33,7 +33,7 @@ typedef struct {
PROCESS_METRIC(METRIC_DECLARE)
} process_metrics_st;

void process_setup(process_options_st *options, process_metrics_st *metrics, struct CDBHandle *cdb_handle);
void process_setup(process_options_st *options, process_metrics_st *metrics, struct cdb_handle *cdb_handle);
void process_teardown(void);

int cdb_process_read(struct buf **rbuf, struct buf **wbuf, void **data);
Expand Down
19 changes: 13 additions & 6 deletions src/server/cdb/main.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "admin/process.h"
#include "setting.h"
#include "stats.h"
#include "storage/cdb/cdb.h"
#include "storage/cdb/cdb_rs.h"

#include "time/time.h"
#include "util/util.h"
Expand Down Expand Up @@ -80,17 +80,24 @@ teardown(void)
log_teardown();
}

static struct CDBHandle*
static struct cdb_handle *
setup_cdb_handle(cdb_options_st *opt)
{
cdb_setup();

char *cdb_file_path = opt->cdb_file_path.val.vstr;
if (cdb_file_path == NULL) {
struct cdb_handle_create_config cfg;
bstring_init(cfg.path);

cfg.load_method = (opt->use_mmap.val.vbool) ? CDB_MMAP : CDB_HEAP;

bstring_set_text(cfg.path, opt->cdb_file_path.val.vstr);

if (cfg.path == NULL) {
log_stderr("cdb_file_path option not set, cannot continue");
exit(EX_CONFIG);
}
return cdb_handle_create(cdb_file_path);

return cdb_handle_create(&cfg);
}

static void
Expand Down Expand Up @@ -140,7 +147,7 @@ setup(void)
compose_setup(NULL, &stats.compose_rsp);
klog_setup(&setting.klog, &stats.klog);

struct CDBHandle* cdb_handle = setup_cdb_handle(&setting.cdb);
struct cdb_handle *cdb_handle = setup_cdb_handle(&setting.cdb);
if (cdb_handle == NULL) {
log_stderr("failed to set up cdb");
goto error;
Expand Down
17 changes: 9 additions & 8 deletions src/server/cdb/setting.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@
#include <channel/cc_tcp.h>
#include <stream/cc_sockio.h>

/* option related */
/* name type default description */
#define CDB_OPTION(ACTION) \
ACTION( daemonize, OPTION_TYPE_BOOL, false, "daemonize the process" )\
ACTION( pid_filename, OPTION_TYPE_STR, NULL, "file storing the pid" )\
ACTION( cdb_file_path, OPTION_TYPE_STR, "db.cdb", "location of the .cdb file" )\
ACTION( dlog_intvl, OPTION_TYPE_UINT, 500, "debug log flush interval(ms)" )\
ACTION( klog_intvl, OPTION_TYPE_UINT, 100, "cmd log flush interval(ms)" )
/* option related */
/* name type default description */
#define CDB_OPTION(ACTION) \
ACTION( daemonize, OPTION_TYPE_BOOL, false, "daemonize the process" )\
ACTION( pid_filename, OPTION_TYPE_STR, NULL, "file storing the pid" )\
ACTION( cdb_file_path, OPTION_TYPE_STR, "db.cdb", "location of the .cdb file" )\
ACTION( use_mmap, OPTION_TYPE_BOOL, false, "use mmap to load the file, false: use the heap" )\
ACTION( dlog_intvl, OPTION_TYPE_UINT, 500, "debug log flush interval(ms)" )\
ACTION( klog_intvl, OPTION_TYPE_UINT, 100, "cmd log flush interval(ms)" )

typedef struct {
CDB_OPTION(OPTION_DECLARE)
Expand Down
2 changes: 2 additions & 0 deletions src/storage/cdb/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@
*.cdb
perf.data*
*.cdb*
.cargo/
dist/
5 changes: 4 additions & 1 deletion src/storage/cdb/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
add_subdirectory(cdb_ffi)
execute_process(COMMAND
env "CMAKE_BINARY_DIR=${CMAKE_BINARY_DIR}" bash scripts/build-cargo-config.sh
WORKING_DIRECTORY "$CMAKE_CURRENT_LIST_DIR}")
add_subdirectory(cdb_rs)
add_dependencies(ccommon_rs_static_target ccommon-static)
Loading

0 comments on commit 58bf871

Please sign in to comment.