You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by ke...@apache.org on 2023/03/10 01:14:04 UTC

[skywalking] branch es/noinit created (now 8638d7d940)

This is an automated email from the ASF dual-hosted git repository.

kezhenxu94 pushed a change to branch es/noinit
in repository https://gitbox.apache.org/repos/asf/skywalking.git


      at 8638d7d940 ElasticSearch storage does not check field types when OAP running in `no-init` mode

This branch includes the following new commits:

     new 8638d7d940 ElasticSearch storage does not check field types when OAP running in `no-init` mode

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[skywalking] 01/01: ElasticSearch storage does not check field types when OAP running in `no-init` mode

Posted by ke...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

kezhenxu94 pushed a commit to branch es/noinit
in repository https://gitbox.apache.org/repos/asf/skywalking.git

commit 8638d7d940ac3d43998a30eeb2474326ed4d46ad
Author: kezhenxu94 <ke...@apache.org>
AuthorDate: Fri Mar 10 09:13:48 2023 +0800

    ElasticSearch storage does not check field types when OAP running in `no-init` mode
---
 docs/en/changes/changes.md                         |  3 ++-
 .../plugin/elasticsearch/base/IndexStructures.java | 26 +++++++++++++++++++++-
 .../elasticsearch/base/StorageEsInstaller.java     | 16 +++++++------
 3 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/docs/en/changes/changes.md b/docs/en/changes/changes.md
index 2d47e90c9f..3a27fb4e29 100644
--- a/docs/en/changes/changes.md
+++ b/docs/en/changes/changes.md
@@ -6,8 +6,9 @@
 #### OAP Server
 
 * Fix wrong layer of metric `user error` in DynamoDB monitoring.
+* ElasticSearch storage does not check field types when OAP running in `no-init` mode.
 
 #### Documentation
 
 
-All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/169?closed=1)
\ No newline at end of file
+All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/169?closed=1)
diff --git a/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/IndexStructures.java b/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/IndexStructures.java
index 11c27f47d4..4174e116a2 100644
--- a/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/IndexStructures.java
+++ b/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/IndexStructures.java
@@ -106,6 +106,23 @@ public class IndexStructures {
                                 .containsAllFields(new Fields(mappings));
     }
 
+    /**
+     * Check whether all the fields are already defined regardless of the field types.
+     * When OAP runs in no-init mode, it doesn't care about the field types, it just
+     * cares about whether the data can be ingested without reporting error.
+     * OAP running in init mode should take care of the field types.
+     */
+    public boolean containsFieldNames(String tableName, Mappings mappings) {
+        if (Objects.isNull(mappings) ||
+            CollectionUtils.isEmpty(mappings.getProperties())) {
+            return true;
+        }
+
+        return mappingStructures.containsKey(tableName) &&
+                mappingStructures.get(tableName)
+                        .containsAllFieldNames(new Fields(mappings));
+    }
+
     /**
      * Returns true when the current index setting equals the input.
      */
@@ -125,7 +142,7 @@ public class IndexStructures {
      */
     public static class Fields {
         private final Map<String, Object> properties;
-        private Mappings.Source source;
+        private final Mappings.Source source;
 
         private Fields(Mappings mapping) {
             this.properties = mapping.getProperties();
@@ -159,6 +176,13 @@ public class IndexStructures {
             return true;
         }
 
+        private boolean containsAllFieldNames(Fields fields) {
+            if (this.properties.size() < fields.properties.size()) {
+                return false;
+            }
+            return this.properties.keySet().containsAll(fields.properties.keySet());
+        }
+
         /**
          * Append new fields and update.
          * Properties combine input and exist, update property's attribute, won't remove old one.
diff --git a/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/StorageEsInstaller.java b/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/StorageEsInstaller.java
index 75b6df1dac..5252e3e107 100644
--- a/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/StorageEsInstaller.java
+++ b/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/StorageEsInstaller.java
@@ -90,12 +90,14 @@ public class StorageEsInstaller extends ModelInstaller {
                 Optional<Index> index = esClient.getIndex(tableName);
                 Mappings historyMapping = index.map(Index::getMappings).orElseGet(Mappings::new);
                 structures.putStructure(tableName, historyMapping, index.map(Index::getSettings).orElseGet(HashMap::new));
-                boolean containsMapping = structures.containsMapping(tableName, createMapping(model));
-                // Do not check index settings in the "no-init mode",
-                // because the no-init mode OAP server doesn't take responsibility for index settings.
                 if (RunningMode.isNoInitMode()) {
-                    exist = containsMapping;
+                    // Do not check index settings and field types in the "no-init mode",
+                    // because the no-init mode OAP server doesn't take responsibility for index settings
+                    // or updating field types, it just cares about whether the data can be ingested without
+                    // reporting errors.
+                    exist = structures.containsFieldNames(tableName, createMapping(model));
                 } else {
+                    boolean containsMapping = structures.containsMapping(tableName, createMapping(model));
                     exist = containsMapping && structures.compareIndexSetting(tableName, createSetting(model));
                 }
             }
@@ -105,7 +107,7 @@ public class StorageEsInstaller extends ModelInstaller {
         boolean templateExists = esClient.isExistsTemplate(tableName);
         final Optional<IndexTemplate> template = esClient.getTemplate(tableName);
 
-        if ((templateExists && !template.isPresent()) || (!templateExists && template.isPresent())) {
+        if ((templateExists && template.isEmpty()) || (!templateExists && template.isPresent())) {
             throw new Error("[Bug warning] ElasticSearch client query template result is not consistent. " +
                                 "Please file an issue to Apache SkyWalking.(https://github.com/apache/skywalking/issues)");
         }
@@ -116,12 +118,12 @@ public class StorageEsInstaller extends ModelInstaller {
             structures.putStructure(
                 tableName, template.get().getMappings(), template.get().getSettings()
             );
-            boolean containsMapping = structures.containsMapping(tableName, createMapping(model));
             // Do not check index settings in the "no-init mode",
             // because the no-init mode OAP server doesn't take responsibility for index settings.
             if (RunningMode.isNoInitMode()) {
-                exist = containsMapping;
+                exist = structures.containsFieldNames(tableName, createMapping(model));
             } else {
+                boolean containsMapping = structures.containsMapping(tableName, createMapping(model));
                 exist = containsMapping && structures.compareIndexSetting(tableName, createSetting(model));
             }
         }