-
Notifications
You must be signed in to change notification settings - Fork 173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Omnibus CDB improvements #194
Changes from 6 commits
3023c9c
2abfc30
3853796
2c42973
bf5b2ef
b86eac7
40f7891
4be42ef
90897cb
12c0686
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,3 +47,4 @@ compile_commands.json | |
|
||
*.cmd | ||
core | ||
CMAKE_BINARY_DIR |
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- | ||
This comment was marked as spam.
Sorry, something went wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [affixing even larger prosthetic neckbeard over actual neckbeard] Most shell I've seen does not put spaces around pipes, and I never do. It's not wrong to put spaces around the pipe (and I think some prefer that convention) but I always thought it's kinda amateurish. |
||
cd "$TEMP/pelikan" | ||
|
||
bash ./ci/run.sh |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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> | ||
|
@@ -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); | ||
|
||
|
@@ -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); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. adding these comments here, because github doesn't support inline comments outside of the diff: line 78: we can just use bstring_init to reset value_buf There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, good call! I wasn't 100% sure about the semantics for doing that assignment on 104 so i just did the safe thing :) |
||
|
||
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); | ||
} | ||
|
||
|
@@ -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; | ||
|
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" | ||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. have you tested this? i think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. whoops, you are right. i misread. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i had to try both :)
This comment was marked as spam.
Sorry, something went wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. some suggestions:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Rust has
This comment was marked as spam.
Sorry, something went wrong. |
||
|
||
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 | ||
|
@@ -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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,3 +5,4 @@ | |
*.cdb | ||
perf.data* | ||
*.cdb* | ||
.cargo/ |
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) |
This comment was marked as spam.
Sorry, something went wrong.