rgw: elastic: fixes for supporting Elasticsearch versions >= 5.0 #21852

theanalyst · 2018-05-07T13:13:05Z

This series of commits introduces support for ES clusters>= 5.0 where the string type is deprecated and 6.0 clusters where the type is invalid. We determine the ESversion and then decide on the string type to use, using text type for ES version >= 5.0.

An enum called ESType is introduced having mappings to various ESTypes and this is later used in dump_type. Also introduces structures for ES version info and cluster name info which we parse from ES endpoint, while only the version info is used to determine the string type, there might be use for storing away the es cluster name and uuid for future use.

Fixes: http://tracker.ceph.com/issues/22877

tianshan · 2018-05-08T08:00:26Z

src/rgw/rgw_sync_module_es.cc

@@ -472,9 +578,11 @@ class RGWElasticHandleRemoteObjCBCR : public RGWStatRemoteObjCBCR {
        string path = conf->get_obj_path(bucket_info, key);
        es_obj_metadata doc(sync_env->cct, conf, bucket_info, key, mtime, size, attrs, versioned_epoch);

+        std::map <string, string> hdrs = {{ "content-type", "application/json" }};


Content-Type is better

tianshan · 2018-05-08T08:05:26Z

src/rgw/rgw_sync_module_es.cc


        es_index_settings settings(conf->num_replicas, conf->num_shards);
        es_index_mappings mappings;
+        if (es_info.version >= ESVersion(5,0)) {
+          ldout(sync_env->cct, 0) << "elasticsearch: using text type for string index mappings " << dendl;
+          mappings.string_type = ESType::Text;


maybe Keyword is enough, we don't need analyse most field

I guess for id like fields we can go for keyword, text is still better for names, like the object/bucket names since keywords expect the exact search term

yehudasa

@theanalyst looking good, just minor comments

yehudasa · 2018-05-08T11:47:49Z

src/rgw/rgw_cr_rest.h

@@ -264,6 +264,17 @@ class RGWPutRESTResourceCR : public RGWSendRESTResourceCR<S, T> {
    : RGWSendRESTResourceCR<S, T>(_cct, _conn, _http_manager,
                                  "PUT", _path,
                                  _params, nullptr, _input, _result) {}
+
+ RGWPutRESTResourceCR(CephContext *_cct, RGWRESTConn *_conn,


@theanalyst indentation maybe?

yehudasa · 2018-05-08T11:51:17Z

src/rgw/rgw_sync_module_es.cc

+bool operator >= (const ESVersion& v1, const ESVersion& v2){
+  if (v1.major_ver > v2.major_ver)
+    return true;
+  else if (v1.major_ver == v2.major_ver)


nit: could do it shorter:

if (v1.major_ver == v2.major_ver) return v1.minor_ver >= v2.minor_ver; return v1.major_ver > v2.major_ver;

yehudasa · 2018-05-08T11:55:51Z

@theanalyst do we have a tracker issue for this one? will need to set it for backport

tchaikov · 2018-05-09T15:27:22Z

src/rgw/rgw_sync_module_es.cc

+  int major_ver;
+  int minor_ver;
+
+  ESVersion(int _major, int _minor): major_ver(_major), minor_ver(_minor) {};


could remove the trailing ;.

tchaikov · 2018-05-09T15:27:29Z

src/rgw/rgw_sync_module_es.cc

+  int minor_ver;
+
+  ESVersion(int _major, int _minor): major_ver(_major), minor_ver(_minor) {};
+  ESVersion(): major_ver(0), minor_ver(0) {};


tchaikov · 2018-05-09T15:27:48Z

src/rgw/rgw_sync_module_es.cc

+  ESVersion(int _major, int _minor): major_ver(_major), minor_ver(_minor) {};
+  ESVersion(): major_ver(0), minor_ver(0) {};
+
+  void from_str(const char* s){


add a space before {.

tchaikov · 2018-05-09T15:28:02Z

src/rgw/rgw_sync_module_es.cc

+    sscanf(s, "%d.%d", &major_ver, &minor_ver);
+  }
+
+  std::string to_str(){


could mark this method const.

tchaikov · 2018-05-09T15:29:56Z

src/rgw/rgw_sync_module_es.cc

@@ -161,13 +209,47 @@ struct ElasticConfig {

 using ElasticConfigRef = std::shared_ptr<ElasticConfig>;

+std::optional<const char*> es_type_to_str(const ESType& t) {
+  switch(t) {


add a space after switch.

tchaikov · 2018-05-09T15:30:35Z

src/rgw/rgw_sync_module_es.cc


  es_dump_type(const char *t, const char *f = nullptr, bool a = false) : type(t), format(f), analyzed(a) {}

+  es_dump_type(ESType t, const char *f=nullptr, bool a = false,


could be more consistent regarding to adding spaces around =

tchaikov · 2018-05-09T15:31:53Z

src/rgw/rgw_sync_module_es.h

+  Text,
+  Keyword,
+
+  /*Numeric Types */


add a space after *

This commit introduces an enum mapping various ES types. Also encode_json now has a new member for string type so that this can be changed easily when upgrading ES versions for eg. wherein new ES engines do not support the string type anymore. Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

Adding another constructor overload for cases when we need to override the default headers Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

Getting the basic information from elasticsearch, while we're only currently interested in elasic search version, other data such as the ES cluster name etc may be useful in the future as well. Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

from Elasticsearch 5.0+ the type string is deprecated, while it is still allowed in 5.0, it returns a 400 error on 6.0 clusters as this type is fully removed. We now determine the es version while initializing the cluster from elasticsearch's default endpoint and use that to determine what string type to use. This way we support both 2.x and 5.x,6.x es versions as we default to string type for clusters < 5.0 Fixes: http://tracker.ceph.com/issues/22877 Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

theanalyst · 2018-05-14T09:36:32Z

jenkins test this please (dashboard related failures)

yuriw · 2018-07-04T23:51:46Z

@theanalyst pls rebase

--- pr 21852 --- pulling https://github.com/theanalyst/ceph.git branch wip-es6-fixes
remote: Counting objects: 21, done.
remote: Total 21 (delta 17), reused 17 (delta 17), pack-reused 4
Unpacking objects: 100% (21/21), done.
From https://github.com/theanalyst/ceph

branch wip-es6-fixes -> FETCH_HEAD
Auto-merging src/rgw/rgw_sync_module_es.cc
CONFLICT (file/directory): There is a directory with name src/dmclock in 2ac2697. Adding src/dmclock as src/dmclock~HEAD
Adding qa/suites/ceph-deploy/basic/distros/ubuntu_latest.yaml
Adding qa/suites/ceph-deploy/basic/distros/centos_latest.yaml
Adding qa/suites/ceph-deploy/basic/distros/.qa
Automatic merge failed; fix conflicts and then commit the result.
Traceback (most recent call last):
File "/home/yuriw/wip_master/src/script/build-integration-branch", line 62, in
assert not r
AssertionError

theanalyst · 2018-08-07T15:40:57Z

This is superceded by #22030

theanalyst requested a review from yehudasa May 7, 2018 13:13

tchaikov added the rgw label May 7, 2018

tianshan reviewed May 8, 2018

View reviewed changes

yehudasa approved these changes May 8, 2018

View reviewed changes

theanalyst force-pushed the wip-es6-fixes branch 2 times, most recently from fb824ed to 6393b5b Compare May 8, 2018 13:15

tchaikov reviewed May 9, 2018

View reviewed changes

theanalyst force-pushed the wip-es6-fixes branch from 6393b5b to ec3dcfa Compare May 11, 2018 08:40

theanalyst added 4 commits May 11, 2018 10:41

rgw: rgw_cr_rest: allow headers to be specified in Put CRs

db41cb2

Adding another constructor overload for cases when we need to override the default headers Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>

theanalyst force-pushed the wip-es6-fixes branch from ec3dcfa to 2ac2697 Compare May 11, 2018 08:42

theanalyst added the needs-qa label May 16, 2018

theanalyst mentioned this pull request May 16, 2018

rgw: es: support username and password for ES #22030

Closed

yuriw added the wip-yuri-testing label Jul 4, 2018

yuriw added wip-yuri-testing and removed wip-yuri-testing needs-qa labels Jul 4, 2018

theanalyst closed this Aug 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rgw: elastic: fixes for supporting Elasticsearch versions >= 5.0 #21852

rgw: elastic: fixes for supporting Elasticsearch versions >= 5.0 #21852

theanalyst commented May 7, 2018 •

edited

Loading

tianshan May 8, 2018

theanalyst May 8, 2018

tianshan May 8, 2018

theanalyst May 8, 2018

yehudasa left a comment

yehudasa May 8, 2018

yehudasa May 8, 2018

yehudasa commented May 8, 2018

tchaikov May 9, 2018

tchaikov May 9, 2018

tchaikov May 9, 2018

tchaikov May 9, 2018

tchaikov May 9, 2018

tchaikov May 9, 2018

tchaikov May 9, 2018

theanalyst commented May 14, 2018

yuriw commented Jul 4, 2018

theanalyst commented Aug 7, 2018


		es_dump_type(const char t, const char f = nullptr, bool a = false) : type(t), format(f), analyzed(a) {}

		es_dump_type(ESType t, const char *f=nullptr, bool a = false,

rgw: elastic: fixes for supporting Elasticsearch versions >= 5.0 #21852

rgw: elastic: fixes for supporting Elasticsearch versions >= 5.0 #21852

Conversation

theanalyst commented May 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yehudasa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yehudasa commented May 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theanalyst commented May 14, 2018

yuriw commented Jul 4, 2018

theanalyst commented Aug 7, 2018

theanalyst commented May 7, 2018 •

edited

Loading