(2010/04/14 1:06), Trond Norbye wrote:
>
> On 13. apr. 2010, at 01.35, KaiGai Kohei wrote:
>
>>
>>> I changed the signature for the existing "get_info" call to allow the engine
>>> to return an arbitrary number of features it support.
>>> If I remember correctly from your proposal you proposed this as a bitmask?
>>> Since this isn't going to be part of any time/space-critical loops, I think
>>> we shouldn't limit the design to a fixed number of features.
>>
>> Do you have a plan to provide guideline of the feature's signature?
>> Is it informed via feature_info->feature? or need to parse description?
>>
>
> The intention is that we define different features as constants in the
> headerfile. People may however define their own feature sets that they
> want to be able to report, but is meaningless for others to care about...
Sorry, the worth of this new method is to inform what features are supported
in this engine module, so it is completely meaningless if people defines
their own feature sets.
Could you define a set of features in the core, like error code?
And, I don't think description for each features are necessary, because it
should be obvious when we see the feature code.
Example)
typedef enum {
ENGINE_FEATURE_CAS = 0, /**< has compare-and-set operation
*/
ENGINE_FEATURE_PERSISTENT_STORAGE, /**< has persistent storage support
*/
ENGINE_FEATURE_SECONDARY_ENGINE, /**< performs as pseudo engine */
ENGINE_FEATURE_ACCESS_CONTROL, /**< has access control feature */
NUM_OF_ENGINE_FEATURES,
} ENGINE_FEATURE_CODE;
Perhaps, # of features is changed depending on initialization option.
If "use_cas=false" will be given, the default_engine does not need to
return ENGINE_FEATURE_CAS, and so on.
>>> To make it even more "flexible" I decided to pass this as an array of
>>> features,
>>> each containing a value and a description.
>>>
>>> The next thing we need to do is to start "registering" different features
>>> :-)
>>
>> In your tree, default_engine.c and corresponding files are deployed at the
>> top level directory... I think an engine module has an individual directory
>> to store corresponding files, when we register different features.
>
> The default engine is special in the way that it is bundled with memcached.
> Other engines should be developed in separate git repositories...
Hmm. It seems to me I misunderstood the meaning of the "registering" different
features.
Thanks,
--
KaiGai Kohei <[email protected]>
--
To unsubscribe, reply using "remove me" as the subject.
diff --git a/default_engine.c b/default_engine.c
index d608cc6..e463163 100644
--- a/default_engine.c
+++ b/default_engine.c
@@ -127,6 +127,10 @@ ENGINE_ERROR_CODE create_instance(uint64_t interface,
.factor = 1.25,
.chunk_size = 48,
.item_size_max= 1024 * 1024,
+ },
+ .info = {
+ .description = "Default engine v0.1",
+ .num_features = 0
}
};
@@ -146,12 +150,9 @@ static inline hash_item* get_real_item(item* item) {
}
static const engine_info* default_get_info(ENGINE_HANDLE* handle) {
- static engine_info info = {
- .description = "Default engine v0.1",
- .num_features = 0
- };
+ struct default_engine* se = get_handle(handle);
- return &info;
+ return &se->info;
}
static ENGINE_ERROR_CODE default_initialize(ENGINE_HANDLE* handle,
@@ -163,6 +164,10 @@ static ENGINE_ERROR_CODE default_initialize(ENGINE_HANDLE* handle,
return ret;
}
+ /* fixup feature_info */
+ if (se->config.use_cas)
+ se->info.features[se->info.num_features++] = ENGINE_FEATURE_CAS;
+
ret = assoc_init(se);
if (ret != ENGINE_SUCCESS) {
return ret;
diff --git a/default_engine.h b/default_engine.h
index 4394569..e1d4a2f 100644
--- a/default_engine.h
+++ b/default_engine.h
@@ -120,6 +120,11 @@ struct default_engine {
struct config config;
struct engine_stats stats;
+
+ /**
+ * The feature to be supported in the default_engine
+ */
+ engine_info info;
};
char* item_get_data(const hash_item* item);
diff --git a/include/memcached/engine.h b/include/memcached/engine.h
index ae0cf9a..75b087a 100644
--- a/include/memcached/engine.h
+++ b/include/memcached/engine.h
@@ -62,6 +62,7 @@ extern "C" {
ENGINE_WANT_MORE = 0x09, /**< The engine want more data if the frontend
* have more data available. */
ENGINE_DISCONNECT = 0x0a, /**< Tell the server to disconnect this client */
+ ENGINE_EACCESS = 0x0b, /**< Access control violations */
ENGINE_FAILED = 0xff /**< Generic failue. */
} ENGINE_ERROR_CODE;
@@ -327,6 +328,14 @@ extern "C" {
const char *description;
} feature_info;
+ typedef enum {
+ ENGINE_FEATURE_CAS = 0, /**< has compare-and-set operation */
+ ENGINE_FEATURE_PERSISTENT_STORAGE, /**< has persistent storage support */
+ ENGINE_FEATURE_SECONDARY_ENGINE, /**< performs as pseudo engine */
+ ENGINE_FEATURE_ACCESS_CONTROL, /**< has access control feature */
+ NUM_OF_ENGINE_FEATURES,
+ } ENGINE_FEATURE_CODE;
+
typedef struct {
/**
* Textual description of this engine
@@ -339,7 +348,7 @@ extern "C" {
/**
* An array containing all of the features the engine supports
*/
- feature_info features[1];
+ ENGINE_FEATURE_CODE features[NUM_OF_ENGINE_FEATURES];
} engine_info;
/**
diff --git a/memcached.c b/memcached.c
index f46c53d..ea5df69 100644
--- a/memcached.c
+++ b/memcached.c
@@ -5751,18 +5751,37 @@ static bool load_engine(const char *soname, const char *config_str) {
if (info->num_features > 0) {
settings.extensions.logger->log(EXTENSION_LOG_INFO, NULL,
- "Supplying the following features:\n");
- }
- for (int ii = 0; ii < info->num_features; ++ii) {
- if (info->features[ii].description != NULL) {
- settings.extensions.logger->log(EXTENSION_LOG_INFO, NULL,
- "%s\n", info->features[ii].description);
-
- } else {
- settings.extensions.logger->log(EXTENSION_LOG_INFO, NULL,
- "Unknown feature: %d\n",
- info->features[ii].feature);
+ "Supplying the following features: ");
+ for (int ii = 0; ii < info->num_features; ++ii) {
+ switch (info->features[ii]) {
+ case ENGINE_FEATURE_CAS:
+ settings.extensions.logger->log(EXTENSION_LOG_INFO, NULL,
+ "%scas",
+ ii > 0 ? ", " : "");
+ break;
+ case ENGINE_FEATURE_PERSISTENT_STORAGE:
+ settings.extensions.logger->log(EXTENSION_LOG_INFO, NULL,
+ "%spersistent storage",
+ ii > 0 ? ", " : "");
+ break;
+ case ENGINE_FEATURE_SECONDARY_ENGINE:
+ settings.extensions.logger->log(EXTENSION_LOG_INFO, NULL,
+ "%ssecondary engine",
+ ii > 0 ? ", " : "");
+ break;
+ case ENGINE_FEATURE_ACCESS_CONTROL:
+ settings.extensions.logger->log(EXTENSION_LOG_INFO, NULL,
+ "%saccess control",
+ ii > 0 ? ", " : "");
+ break;
+ default:
+ settings.extensions.logger->log(EXTENSION_LOG_INFO, NULL,
+ "%sunknow feature (code=%u)",
+ ii > 0 ? ", " : "");
+ break;
+ }
}
+ settings.extensions.logger->log(EXTENSION_LOG_INFO, NULL, "\n");
}
} else {
settings.extensions.logger->log(EXTENSION_LOG_INFO, NULL,
