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:05 UTC

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

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));
             }
         }