diff --git a/src/modules/content-files/content-files.c b/src/modules/content-files/content-files.c index b5c3fad8db6e..e38d18dfb864 100644 --- a/src/modules/content-files/content-files.c +++ b/src/modules/content-files/content-files.c @@ -227,8 +227,7 @@ void checkpoint_get_cb (flux_t *h, if (flux_respond_pack (h, msg, "{s:O}", - "value", - o) < 0) + "value", o) < 0) flux_log_error (h, "error responding to checkpoint-get request"); free (data); json_decref (o); @@ -257,10 +256,8 @@ void checkpoint_put_cb (flux_t *h, if (flux_request_unpack (msg, NULL, "{s:s s:o}", - "key", - &key, - "value", - &o) < 0) + "key", &key, + "value", &o) < 0) goto error; if (!(value = json_dumps (o, JSON_COMPACT))) { errstr = "failed to encode checkpoint value"; diff --git a/src/modules/content-s3/content-s3.c b/src/modules/content-s3/content-s3.c index 4f39e140851f..3a7521a0935c 100644 --- a/src/modules/content-s3/content-s3.c +++ b/src/modules/content-s3/content-s3.c @@ -158,14 +158,10 @@ static struct s3_config *parse_config (const flux_conf_t *conf, &error, "{s:{s:s, s:s, s:s, s?b !} }", "content-s3", - "credential-file", - &cred_file, - "bucket", - &bucket, - "uri", - &uri, - "virtual-host-style", - &is_virtual_host) < 0) { + "credential-file", &cred_file, + "bucket", &bucket, + "uri", &uri, + "virtual-host-style", &is_virtual_host) < 0) { errprintf (errp, "%s", error.text); goto error; } @@ -367,8 +363,7 @@ void checkpoint_get_cb (flux_t *h, if (flux_respond_pack (h, msg, "{s:O}", - "value", - o) < 0) { + "value", o) < 0) { errno = EIO; flux_log_error (h, "error responding to checkpoint-get request (pack)"); } @@ -400,10 +395,8 @@ void checkpoint_put_cb (flux_t *h, if (flux_request_unpack (msg, NULL, "{s:s s:o}", - "key", - &key, - "value", - &o) < 0) + "key", &key, + "value", &o) < 0) goto error; if (!(value = json_dumps (o, JSON_COMPACT))) { errstr = "failed to encode checkpoint value"; diff --git a/src/modules/content-sqlite/content-sqlite.c b/src/modules/content-sqlite/content-sqlite.c index 312d8060e1dc..764324915fba 100644 --- a/src/modules/content-sqlite/content-sqlite.c +++ b/src/modules/content-sqlite/content-sqlite.c @@ -413,8 +413,7 @@ void checkpoint_get_cb (flux_t *h, if (flux_respond_pack (h, msg, "{s:O}", - "value", - o) < 0) + "value", o) < 0) flux_log_error (h, "flux_respond_pack"); (void )sqlite3_reset (ctx->checkpt_get_stmt); json_decref (o); @@ -440,10 +439,8 @@ void checkpoint_put_cb (flux_t *h, if (flux_request_unpack (msg, NULL, "{s:s s:o}", - "key", - &key, - "value", - &o) < 0) + "key", &key, + "value", &o) < 0) goto error; if (!(value = json_dumps (o, JSON_COMPACT))) { errstr = "failed to encode checkpoint value"; diff --git a/src/modules/content/cache.c b/src/modules/content/cache.c index b543f397fa7a..fa540cca3b37 100644 --- a/src/modules/content/cache.c +++ b/src/modules/content/cache.c @@ -782,7 +782,6 @@ static void content_register_backing_request (flux_t *h, if (flux_respond (h, msg, NULL) < 0) flux_log_error (h, "error responding to register-backing request"); (void)cache_flush (cache); - (void)checkpoints_flush (cache->checkpoint); return; error: if (flux_respond_error (h, msg, errno, errstr) < 0) diff --git a/src/modules/content/checkpoint.c b/src/modules/content/checkpoint.c index ad181eadef23..b0d0b0c878a3 100644 --- a/src/modules/content/checkpoint.c +++ b/src/modules/content/checkpoint.c @@ -28,74 +28,8 @@ struct content_checkpoint { flux_msg_handler_t **handlers; uint32_t rank; struct content_cache *cache; - zhashx_t *hash; - unsigned int hash_dirty; }; -struct checkpoint_data { - struct content_checkpoint *checkpoint; - json_t *value; - uint8_t dirty:1; - bool in_progress; - int refcount; -}; - -static struct checkpoint_data * -checkpoint_data_incref (struct checkpoint_data *data) -{ - if (data) - data->refcount++; - return data; -} - -static void checkpoint_data_decref (struct checkpoint_data *data) -{ - if (data && --data->refcount == 0) { - if (data->dirty) - data->checkpoint->hash_dirty--; - json_decref (data->value); - free (data); - } -} - -/* zhashx_destructor_fn */ -static void checkpoint_data_decref_wrapper (void **arg) -{ - if (arg) { - struct checkpoint_data *data = *arg; - checkpoint_data_decref (data); - } -} - -static struct checkpoint_data * -checkpoint_data_create (struct content_checkpoint *checkpoint, - json_t *value) -{ - struct checkpoint_data *data = NULL; - - if (!(data = calloc (1, sizeof (*data)))) - return NULL; - data->checkpoint = checkpoint; - data->value = json_incref (value); - data->refcount = 1; - return data; -} - -static int checkpoint_data_update (struct content_checkpoint *checkpoint, - const char *key, - json_t *value) -{ - struct checkpoint_data *data = NULL; - - if (!(data = checkpoint_data_create (checkpoint, value))) - return -1; - - zhashx_update (checkpoint->hash, key, data); - data->dirty = 1; - checkpoint->hash_dirty++; - return 0; -} - static void checkpoint_get_continuation (flux_future_t *f, void *arg) { struct content_checkpoint *checkpoint = arg; @@ -111,9 +45,6 @@ static void checkpoint_get_continuation (flux_future_t *f, void *arg) if (flux_rpc_get_unpack (f, "{s:o}", "value", &value) < 0) goto error; - if (checkpoint_data_update (checkpoint, key, value) < 0) - goto error; - if (flux_respond_pack (checkpoint->h, msg, "{s:O}", "value", value) < 0) flux_log_error (checkpoint->h, "error responding to checkpoint-get"); @@ -176,24 +107,16 @@ void content_checkpoint_get_request (flux_t *h, flux_msg_handler_t *mh, const char *key; const char *errstr = NULL; - if (flux_request_unpack (msg, NULL, "{s:s}", "key", &key) < 0) - goto error; - if (checkpoint->rank == 0 && !content_cache_backing_loaded (checkpoint->cache)) { - struct checkpoint_data *data = zhashx_lookup (checkpoint->hash, key); - if (!data) { - errstr = "checkpoint key unavailable"; - errno = ENOENT; - goto error; - } - if (flux_respond_pack (h, msg, - "{s:O}", - "value", data->value) < 0) - flux_log_error (h, "error responding to checkpoint-get"); - return; + errstr = "checkpoint get unavailable, no backing store"; + errno = ENOSYS; + goto error; } + if (flux_request_unpack (msg, NULL, "{s:s}", "key", &key) < 0) + goto error; + if (checkpoint_get_forward (checkpoint, msg, key, @@ -279,26 +202,20 @@ void content_checkpoint_put_request (flux_t *h, flux_msg_handler_t *mh, json_t *value; const char *errstr = NULL; + if (checkpoint->rank == 0 + && !content_cache_backing_loaded (checkpoint->cache)) { + errstr = "checkpoint put unavailable, no backing store"; + errno = ENOSYS; + goto error; + } + if (flux_request_unpack (msg, NULL, "{s:s s:o}", - "key", - &key, - "value", - &value) < 0) + "key", &key, + "value", &value) < 0) goto error; - if (checkpoint->rank == 0) { - if (checkpoint_data_update (checkpoint, key, value) < 0) - goto error; - - if (!content_cache_backing_loaded (checkpoint->cache)) { - if (flux_respond (h, msg, NULL) < 0) - flux_log_error (checkpoint->h, "error responding to checkpoint-put"); - return; - } - } - if (checkpoint_put_forward (checkpoint, msg, key, @@ -313,72 +230,6 @@ void content_checkpoint_put_request (flux_t *h, flux_msg_handler_t *mh, flux_log_error (h, "error responding to checkpoint-put request"); } -static void checkpoint_flush_continuation (flux_future_t *f, void *arg) -{ - struct checkpoint_data *data = arg; - int rv; - - assert (data); - if ((rv = flux_rpc_get (f, NULL)) < 0) - flux_log_error (data->checkpoint->h, "checkpoint flush rpc"); - if (!rv) { - data->dirty = 0; - data->checkpoint->hash_dirty--; - } - data->in_progress = false; - checkpoint_data_decref (data); - flux_future_destroy (f); -} - -static int checkpoint_flush (struct content_checkpoint *checkpoint, - struct checkpoint_data *data) -{ - if (data->dirty && !data->in_progress) { - const char *key = zhashx_cursor (checkpoint->hash); - const char *topic = "content-backing.checkpoint-put"; - flux_future_t *f; - if (!(f = flux_rpc_pack (checkpoint->h, topic, 0, 0, - "{s:s s:O}", - "key", key, - "value", data->value)) - || flux_future_then (f, - -1, - checkpoint_flush_continuation, - (void *)checkpoint_data_incref (data)) < 0) { - flux_log_error (checkpoint->h, "%s: checkpoint flush", __FUNCTION__); - flux_future_destroy (f); - return -1; - } - data->in_progress = true; - } - return 0; -} - -int checkpoints_flush (struct content_checkpoint *checkpoint) -{ - int last_errno = 0; - int rc = 0; - - if (checkpoint->hash_dirty > 0) { - struct checkpoint_data *data = zhashx_first (checkpoint->hash); - while (data) { - if (checkpoint_flush (checkpoint, data) < 0) { - last_errno = errno; - rc = -1; - /* A few errors we will consider "unrecoverable", so - * break out */ - if (errno == ENOSYS - || errno == ENOMEM) - break; - } - data = zhashx_next (checkpoint->hash); - } - } - if (rc < 0) - errno = last_errno; - return rc; -} - static const struct flux_msg_handler_spec htab[] = { { FLUX_MSGTYPE_REQUEST, @@ -400,7 +251,6 @@ void content_checkpoint_destroy (struct content_checkpoint *checkpoint) if (checkpoint) { int saved_errno = errno; flux_msg_handler_delvec (checkpoint->handlers); - zhashx_destroy (&checkpoint->hash); free (checkpoint); errno = saved_errno; } @@ -419,16 +269,10 @@ struct content_checkpoint *content_checkpoint_create ( checkpoint->rank = rank; checkpoint->cache = cache; - if (!(checkpoint->hash = zhashx_new ())) - goto nomem; - zhashx_set_destructor (checkpoint->hash, checkpoint_data_decref_wrapper); - if (flux_msg_handler_addvec (h, htab, checkpoint, &checkpoint->handlers) < 0) goto error; return checkpoint; -nomem: - errno = ENOMEM; error: content_checkpoint_destroy (checkpoint); return NULL; diff --git a/src/modules/content/checkpoint.h b/src/modules/content/checkpoint.h index 7bd0c5ad6622..d0322ce6b40e 100644 --- a/src/modules/content/checkpoint.h +++ b/src/modules/content/checkpoint.h @@ -19,8 +19,6 @@ struct content_checkpoint *content_checkpoint_create ( struct content_cache *cache); void content_checkpoint_destroy (struct content_checkpoint *checkpoint); -int checkpoints_flush (struct content_checkpoint *checkpoint); - #endif /* !_CONTENT_CHECKPOINT_H */ /* diff --git a/t/t0012-content-sqlite.t b/t/t0012-content-sqlite.t index f94fba428320..4d61317b8fe4 100755 --- a/t/t0012-content-sqlite.t +++ b/t/t0012-content-sqlite.t @@ -227,47 +227,8 @@ test_expect_success 'remove content-sqlite module on rank 0' ' flux module remove content-sqlite ' -test_expect_success 'checkpoint-put foo w/ rootref spoon' ' - checkpoint_put foo spoon -' - -test_expect_success 'checkpoint-get foo returned rootref spoon' ' - echo spoon >rootref5.exp && - checkpoint_get foo | jq -r .value | jq -r .rootref >rootref5.out && - test_cmp rootref5.exp rootref5.out -' - -test_expect_success 'load content-sqlite module on rank 0' ' - flux module load content-sqlite -' - -# arg1 - expected reference -wait_checkpoint_flush() { - local expected=$1 - local i=0 - while checkpoint_backing_get foo \ - | jq -r .value \ - | jq -r .rootref > checkpointflush.out \ - && [ $i -lt 50 ] - do - checkpoint=$(cat checkpointflush.out) - if [ "${checkpoint}" = "${expected}" ] - then - return 0 - fi - sleep 0.1 - i=$((i + 1)) - done - return 1 -} - -test_expect_success 'checkpoint-backing-get foo returns spoon' ' - wait_checkpoint_flush spoon -' - -test_expect_success 'remove content-sqlite module on rank 0' ' - flux content flush && - flux module remove content-sqlite +test_expect_success 'checkpoint-put foo w/ rootref spoon fails without backing' ' + test_must_fail checkpoint_put foo spoon ' test_expect_success 'remove heartbeat module' ' diff --git a/t/t0018-content-files.t b/t/t0018-content-files.t index b0004d4cf18a..3a69affdea46 100755 --- a/t/t0018-content-files.t +++ b/t/t0018-content-files.t @@ -249,56 +249,17 @@ test_expect_success 'checkpoint-put bad request fails with EPROTO' ' grep "Protocol error" badput.err ' -test_expect_success 'remove content-files module' ' - flux module remove content-files -' - -test_expect_success 'checkpoint-put foo w/ rootref spoon' ' - checkpoint_put foo spoon -' - -test_expect_success 'checkpoint-get foo returned rootref spoon' ' - echo spoon >rootref7.exp && - checkpoint_get foo | jq -r .value | jq -r .rootref >rootref7.out && - test_cmp rootref7.exp rootref7.out -' - -test_expect_success 'load content-files module on rank 0' ' - flux module load content-files -' - -# arg1 - expected reference -wait_checkpoint_flush() { - local expected=$1 - local i=0 - while checkpoint_backing_get foo \ - | jq -r .value \ - | jq -r .rootref > checkpointflush.out \ - && [ $i -lt 50 ] - do - checkpoint=$(cat checkpointflush.out) - if [ "${checkpoint}" = "${expected}" ] - then - return 0 - fi - sleep 0.1 - i=$((i + 1)) - done - return 1 -} - -test_expect_success 'checkpoint-backing-get foo returns spoon' ' - wait_checkpoint_flush spoon -' - test_expect_success 'flux module stats content-files is open to guests' ' FLUX_HANDLE_ROLEMASK=0x2 \ flux module stats content-files >/dev/null ' -test_expect_success 'remove content-files module on rank 0' ' - flux content flush && - flux module remove content-files +test_expect_success 'remove content-files module' ' + flux module remove content-files +' + +test_expect_success 'checkpoint-put foo w/ rootref spoon fails without backing' ' + test_must_fail checkpoint_put foo spoon ' test_expect_success 'remove content module' ' diff --git a/t/t0028-content-backing-none.t b/t/t0028-content-backing-none.t index 129cd5404114..8bd824baf93c 100755 --- a/t/t0028-content-backing-none.t +++ b/t/t0028-content-backing-none.t @@ -15,80 +15,35 @@ test_expect_success 'loaded content module' ' flux exec flux module load content ' -test_expect_success 'checkpoint-get fails, no checkpoints yet' ' - checkpoint_put foo bar +test_expect_success 'checkpoint-get fails, no backing store' ' + test_must_fail checkpoint_get foo ' -test_expect_success 'checkpoint-put foo w/ rootref bar' ' - checkpoint_put foo bar +test_expect_success 'checkpoint-put fails, no backing store' ' + test_must_fail checkpoint_put foo bar ' -test_expect_success 'checkpoint-get foo returned rootref bar' ' - echo bar >rootref.exp && - checkpoint_get foo | jq -r .value | jq -r .rootref >rootref.out && - test_cmp rootref.exp rootref.out -' - -test_expect_success 'checkpoint-put on rank 1 forwards to rank 0' ' - o=$(checkpoint_put_msg rankone rankref) && - jq -j -c -n ${o} | flux exec -r 1 ${RPC} content.checkpoint-put -' - -test_expect_success 'checkpoint-get on rank 1 forwards to rank 0' ' - echo rankref >rankref.exp && - o=$(checkpoint_get_msg rankone) && - jq -j -c -n ${o} \ - | flux exec -r 1 ${RPC} content.checkpoint-get \ - | jq -r .value | jq -r .rootref > rankref.out && - test_cmp rankref.exp rankref.out -' - -test_expect_success 'flux-dump --checkpoint with missing checkpoint fails' ' +test_expect_success 'flux-dump --checkpoint with no backing store' ' test_must_fail flux dump --checkpoint foo.tar ' -test_expect_success 'load kvs and create some kvs data' ' - flux module load kvs && - flux kvs put a=1 && - flux kvs put b=foo -' - -test_expect_success 'reload kvs' ' - flux module reload kvs && - test $(flux kvs get a) = "1" && - test $(flux kvs get b) = "foo" -' - -test_expect_success 'unload kvs' ' - flux module remove kvs -' - -test_expect_success 'dump default=kvs-primary checkpoint works' ' - flux dump --checkpoint foo.tar -' - -test_expect_success 'restore content' ' - flux restore --checkpoint foo.tar -' - -test_expect_success 'reload kvs' ' - flux module load kvs +test_expect_success 'content.backing-module input of none works' ' + flux start -Scontent.backing-module=none true ' -test_expect_success 'verify KVS content restored' ' - test $(flux kvs get a) = "1" && - test $(flux kvs get b) = "foo" +test_expect_success 'checkpoint-get fails, backing store is none' ' + test_must_fail checkpoint_get foo ' -test_expect_success 'unload kvs' ' - flux module remove kvs +test_expect_success 'checkpoint-put fails, backing store is none' ' + test_must_fail checkpoint_put foo bar ' -test_expect_success 'content.backing-module input of none works' ' - flux start -Scontent.backing-module=none true +test_expect_success 'flux-dump --checkpoint with backing store is none' ' + test_must_fail flux dump --checkpoint foo.tar ' -test_expect_success 'removedcontent module' ' +test_expect_success 'remove content module' ' flux exec flux module remove content ' diff --git a/t/t2807-dump-cmd.t b/t/t2807-dump-cmd.t index e8092315f139..c4abafc26e35 100755 --- a/t/t2807-dump-cmd.t +++ b/t/t2807-dump-cmd.t @@ -27,7 +27,7 @@ test_expect_success 'flux-restore with no args prints Usage message' ' ' test_expect_success 'flux-dump with no backing store fails' ' test_must_fail flux dump --checkpoint foo.tar 2>nostore.err && - grep "checkpoint key unavailable" nostore.err + grep "checkpoint get unavailable, no backing store" nostore.err ' test_expect_success 'flux-dump with bad archive file fails' ' test_must_fail flux dump /badfile.tar 2>badfile.err && @@ -181,9 +181,9 @@ test_expect_success 'restore to key fails when kvs is not loaded' ' test_expect_success 'unload content-sqlite' ' flux module remove content-sqlite ' -test_expect_success 'restore --checkpoint with no backing store cant flush' ' - flux restore --checkpoint foo.tar 2>noback.err && - grep "error flushing content cache" noback.err +test_expect_success 'restore --checkpoint with no backing store cant checkpoint' ' + test_must_fail flux restore --checkpoint foo.tar 2>noback.err && + grep "checkpoint put unavailable, no backing store" noback.err ' test_expect_success 'dump --no-cache with no backing store fails' ' test_must_fail flux dump --no-cache --checkpoint x.tar