From f508521ff9989a7b3d7753bdc9636e35b20f63a5 Mon Sep 17 00:00:00 2001 From: Stephanie Date: Thu, 14 Feb 2019 15:39:45 -0800 Subject: [PATCH] Use Redis auto memory management --- src/ray/gcs/redis_module/ray_redis_module.cc | 34 ++++++++++---------- src/ray/gcs/tables.cc | 2 ++ 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/ray/gcs/redis_module/ray_redis_module.cc b/src/ray/gcs/redis_module/ray_redis_module.cc index 42a910fa0bea4..670597fa28c80 100644 --- a/src/ray/gcs/redis_module/ray_redis_module.cc +++ b/src/ray/gcs/redis_module/ray_redis_module.cc @@ -40,10 +40,10 @@ extern RedisChainModule module; } // Wrap a Redis command with automatic memory management. -#define AUTO_MEMORY(FUNC) \ +#define AUTO_MEMORY(FUNC) \ int FUNC(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { \ - RedisModule_AutoMemory(ctx); \ - return redis_commands::FUNC(ctx, argv, argc); \ + RedisModule_AutoMemory(ctx); \ + return redis_commands::FUNC(ctx, argv, argc); \ } /// Commands in this namespace can be wrapped with AUTO_MEMORY in the @@ -682,28 +682,28 @@ int DebugString_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int std::string debug_string = DebugString(); return RedisModule_ReplyWithStringBuffer(ctx, debug_string.data(), debug_string.size()); } - }; - namespace auto_memory_redis_commands { - // Wrap all Redis commands with Redis' auto memory management. - AUTO_MEMORY(TableAdd_RedisCommand); - AUTO_MEMORY(TableAppend_RedisCommand); - AUTO_MEMORY(TableLookup_RedisCommand); - AUTO_MEMORY(TableRequestNotifications_RedisCommand); - AUTO_MEMORY(TableCancelNotifications_RedisCommand); - AUTO_MEMORY(TableTestAndUpdate_RedisCommand); - AUTO_MEMORY(DebugString_RedisCommand); +// Wrap all Redis commands with Redis' auto memory management. +AUTO_MEMORY(TableAdd_RedisCommand); +AUTO_MEMORY(TableAppend_RedisCommand); +AUTO_MEMORY(TableLookup_RedisCommand); +AUTO_MEMORY(TableRequestNotifications_RedisCommand); +AUTO_MEMORY(TableCancelNotifications_RedisCommand); +AUTO_MEMORY(TableTestAndUpdate_RedisCommand); +AUTO_MEMORY(DebugString_RedisCommand); #if RAY_USE_NEW_GCS - AUTO_MEMORY(ChainTableAdd_RedisCommand); - AUTO_MEMORY(ChainTableAppend_RedisCommand); +AUTO_MEMORY(ChainTableAdd_RedisCommand); +AUTO_MEMORY(ChainTableAppend_RedisCommand); #endif }; extern "C" { - // Only use commands that have auto memory management. - using namespace auto_memory_redis_commands; +// Only use commands that have auto memory management. +// TODO(swang): Ideally, we would make the commands that don't have auto memory +// management inaccessible instead of just using separate namespaces. +using namespace auto_memory_redis_commands; /* This function must be present on each Redis module. It is used in order to * register the commands into the Redis server. */ diff --git a/src/ray/gcs/tables.cc b/src/ray/gcs/tables.cc index 8fc96b9d5d2e6..c1342c159f7d4 100644 --- a/src/ray/gcs/tables.cc +++ b/src/ray/gcs/tables.cc @@ -42,7 +42,9 @@ Status Log::Append(const JobID &job_id, const ID &id, std::shared_ptr &dataT, const WriteCallback &done) { num_appends_++; auto callback = [this, id, dataT, done](const std::string &data) { + // If data is not empty, then Redis failed to append the entry. RAY_CHECK(data.empty()) << "TABLE_APPEND command failed: " << data; + if (done != nullptr) { (done)(client_, id, *dataT); }