From a105b25915a2b9c900957c71443e0be2ef5fa9ec Mon Sep 17 00:00:00 2001 From: Amar Tumballi Date: Fri, 27 Jul 2018 23:11:51 +0530 Subject: classification: provide infra to start labelling features/components `doc/xlator-classification.md` talks about the reasoning and expectations Reviewers are expected to check the 'category' of new option / translator added in the codebase, and make sure the flag is always properly set. It helps to keep the 'expectation' proper on the codebase. updates: #430 Change-Id: I2bfc9934a5f6eed77fcc3e20364046242decc82c Signed-off-by: Amar Tumballi --- doc/developer-guide/xlator-classification.md | 45 +++++++++++++++++++++++++++- extras/create_new_xlator/new-xlator-tmpl.c | 4 ++- libglusterfs/src/glusterfs.h | 29 ++++++++++++++++++ libglusterfs/src/options.h | 12 ++++++-- libglusterfs/src/xlator.c | 2 ++ libglusterfs/src/xlator.h | 6 +++- 6 files changed, 92 insertions(+), 6 deletions(-) diff --git a/doc/developer-guide/xlator-classification.md b/doc/developer-guide/xlator-classification.md index 50d2bdc61e8..36d924d0934 100644 --- a/doc/developer-guide/xlator-classification.md +++ b/doc/developer-guide/xlator-classification.md @@ -130,7 +130,50 @@ These xlators and their corresponding code and test health will not be executed. ### How to specify an xlators category -(TBD) +While defining 'xlator_api_t' structure for the corresponding xlator, add a +flag like below: + +``` +diff --git a/xlators/performance/nl-cache/src/nl-cache.c b/xlators/performance/nl-cache/src/nl-cache.c +index 0f0e53bac2..8267d6897c 100644 +--- a/xlators/performance/nl-cache/src/nl-cache.c ++++ b/xlators/performance/nl-cache/src/nl-cache.c +@@ -869,4 +869,5 @@ xlator_api_t xlator_api = { + .cbks = &nlc_cbks, + .options = nlc_options, + .identifier = "nl-cache", ++ .category = GF_TECH_PREVIEW, + }; +diff --git a/xlators/performance/quick-read/src/quick-read.c b/xlators/performance/quick-read/src/quick-read.c +index 8d39720e7f..235de27c19 100644 +--- a/xlators/performance/quick-read/src/quick-read.c ++++ b/xlators/performance/quick-read/src/quick-read.c +@@ -1702,4 +1702,5 @@ xlator_api_t xlator_api = { + .cbks = &qr_cbks, + .options = qr_options, + .identifier = "quick-read", ++ .category = GF_MAINTAINED, + }; +``` + +Similarly, if a particular option is in different state other than +the xlator state, one can add the same flag in options structure too. + +``` +diff --git a/xlators/cluster/afr/src/afr.c b/xlators/cluster/afr/src/afr.c +index 0e86e33d03..81996743d1 100644 +--- a/xlators/cluster/afr/src/afr.c ++++ b/xlators/cluster/afr/src/afr.c +@@ -772,6 +772,7 @@ struct volume_options options[] = { + .description = "Maximum latency for shd halo replication in msec." + }, + { .key = {"halo-enabled"}, ++ .category = GF_TECH_PREVIEW, + .type = GF_OPTION_TYPE_BOOL, + .default_value = "False", + +``` + ### User experience using the categories diff --git a/extras/create_new_xlator/new-xlator-tmpl.c b/extras/create_new_xlator/new-xlator-tmpl.c index 1f2f9c316a2..474144c1c81 100644 --- a/extras/create_new_xlator/new-xlator-tmpl.c +++ b/extras/create_new_xlator/new-xlator-tmpl.c @@ -96,7 +96,8 @@ struct volume_options @FOP_PREFIX@_options[] = { .op_version = {GD_OP_VERSION_}, .flags = OPT_FLAG_SETTABLE | OPT_FLAG_DOC | OPT_FLAG_CLIENT_OPT, .tags = {""}, - .description = "" + .description = "", + .category = GF_EXPERIMENTAL, }, { .key = {NULL} }, */ @@ -115,6 +116,7 @@ xlator_api_t xlator_api = { .cbks = &@FOP_PREFIX@_cbks, .options = @FOP_PREFIX@_options, .identifier = "@XL_NAME@", + .category = GF_EXPERIMENTAL, }; #pragma fragment HEADER_FMT #ifndef __@HFL_NAME@_H__ diff --git a/libglusterfs/src/glusterfs.h b/libglusterfs/src/glusterfs.h index 423804300a3..31fb65b8660 100644 --- a/libglusterfs/src/glusterfs.h +++ b/libglusterfs/src/glusterfs.h @@ -390,6 +390,35 @@ typedef enum { GF_FOP_PRI_MAX, /* Highest */ } gf_fop_pri_t; +typedef enum { + /* The 'component' (xlator / option) is not yet setting the flag */ + GF_UNCLASSIFIED = 0, + /* The 'component' is experimental, should not be recommened + in production mode */ + GF_EXPERIMENTAL, + /* The 'component' is tech preview, ie, it is 'mostly' working as + expected, but can have some of the corner cases, which is not + handled. */ + GF_TECH_PREVIEW, + /* The 'component' is good to run. Has good enough test and + documentation coverage. */ + GF_MAINTAINED, + /* The component is: + - no more a focus + - no more solving a valid use case + - no more maintained, no volunteers to maintain + - there is 'maintained' or 'tech-preview' feature, + which does the same thing, better. + */ + GF_DEPRECATED, + /* The 'component' is no more 'built'. */ + GF_OBSOLETE, + /* The 'component' exist for Documentation purposes. + No real usecase */ + GF_DOCUMENT_PURPOSE, +} gf_category_t; + + static const char * const FOP_PRI_STRINGS[] = { "HIGH", "NORMAL", diff --git a/libglusterfs/src/options.h b/libglusterfs/src/options.h index 1aa27341624..1ce8975e799 100644 --- a/libglusterfs/src/options.h +++ b/libglusterfs/src/options.h @@ -134,9 +134,15 @@ typedef struct volume_options { */ char *setkey; - /* The level at which the option is classified - */ - opt_level_t level; + /* A 'level' is about the technical depth / understanding one + needs to handle the option. 'category' is based on + quality (ie, tests, people behind it, documentation available) */ + + /* The level at which the option is classified */ + opt_level_t level; + + /* Flag to understand how this option is categorized */ + gf_category_t category; } volume_option_t; diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c index 22e3e041fd9..d3c134ba903 100644 --- a/libglusterfs/src/xlator.c +++ b/libglusterfs/src/xlator.c @@ -417,6 +417,8 @@ int xlator_dynload_newway (xlator_t *xl) xl->id = xlapi->xlator_id; xl->flags = xlapi->flags; xl->identifier = xlapi->identifier; + xl->category = xlapi->category; + memcpy (xl->op_version, xlapi->op_version, sizeof (uint32_t) * GF_MAX_RELEASES); diff --git a/libglusterfs/src/xlator.h b/libglusterfs/src/xlator.h index d476cf26442..e0b8207444c 100644 --- a/libglusterfs/src/xlator.h +++ b/libglusterfs/src/xlator.h @@ -1051,7 +1051,8 @@ struct _xlator { /* flag to avoid recall of xlator_mem_cleanup for xame xlator */ uint32_t call_cleanup; - + /* Flag to understand how this xlator is categorized */ + gf_category_t category; }; typedef struct { @@ -1092,6 +1093,9 @@ typedef struct { volume file, then that should be defined here. optional. */ volume_option_t *options; + /* Flag to understand how this xlator is categorized */ + gf_category_t category; + /* XXX: GD2MARKER * If a new member that needs to be visible to GD2 is introduced, * add it above this comment. -- cgit