You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/11/07 06:16:20 UTC
[GitHub] [skywalking] wankai123 opened a new pull request, #9914: Elasticsearch storage: support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
wankai123 opened a new pull request, #9914:
URL: https://github.com/apache/skywalking/pull/9914
- Elasticsearch storage: Provide system environment variable(`SW_STORAGE_ES_SPECIFIC_INDEX_SETTINGS`) and support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
- Elasticsearch storage: Support update index settings `(number_of_shards/number_of_replicas)` for the index template after re-configured.
- [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
- [ ] Update the documentation to include this new feature.
- [X] Tests(including UT, IT, E2E) are added to verify the new feature.
- [ ] If it's UI related, attach the screenshots below.
- [X] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes (https://github.com/apache/skywalking/issues/9849).
- [X] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/docs/en/changes/changes.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on a diff in pull request #9914: Elasticsearch storage: support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #9914:
URL: https://github.com/apache/skywalking/pull/9914#discussion_r1015058568
##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/StorageEsInstaller.java:
##########
@@ -56,25 +59,31 @@ public StorageEsInstaller(Client client,
this.columnTypeEsMapping = new ColumnTypeEsMapping();
this.config = config;
this.structures = getStructures();
+ if (StringUtil.isNotEmpty(config.getSpecificIndexSettings())) {
+ this.specificIndexesSettings = gson.fromJson(config.getSpecificIndexSettings(), Map.class);
+ } else {
+ this.specificIndexesSettings = Collections.EMPTY_MAP;
+ }
}
protected IndexStructures getStructures() {
return new IndexStructures();
}
@Override
- protected boolean isExists(Model model) {
+ protected boolean isExists(Model model) throws StorageException {
ElasticSearchClient esClient = (ElasticSearchClient) client;
String tableName = IndexController.INSTANCE.getTableName(model);
IndexController.LogicIndicesRegister.registerRelation(model, tableName);
if (!model.isTimeSeries()) {
boolean exist = esClient.isExistsIndex(tableName);
if (exist) {
- Mappings historyMapping = esClient.getIndex(tableName)
- .map(Index::getMappings)
- .orElseGet(Mappings::new);
- structures.putStructure(tableName, historyMapping);
- exist = structures.containsStructure(tableName, createMapping(model));
+ esClient.getIndex(tableName);
Review Comment:
This can be removed.
```suggestion
```
##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/StorageEsInstaller.java:
##########
@@ -56,25 +59,31 @@ public StorageEsInstaller(Client client,
this.columnTypeEsMapping = new ColumnTypeEsMapping();
this.config = config;
this.structures = getStructures();
+ if (StringUtil.isNotEmpty(config.getSpecificIndexSettings())) {
+ this.specificIndexesSettings = gson.fromJson(config.getSpecificIndexSettings(), Map.class);
+ } else {
+ this.specificIndexesSettings = Collections.EMPTY_MAP;
+ }
Review Comment:
A nit to eliminate warnings
```suggestion
if (StringUtil.isNotEmpty(config.getSpecificIndexSettings())) {
this.specificIndexesSettings = gson.fromJson(config.getSpecificIndexSettings(), new TypeReference<Map<String, Map<String, Object>>>() {
}.getType());
} else {
this.specificIndexesSettings = Collections.emptyMap();
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9914: Elasticsearch storage: support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9914:
URL: https://github.com/apache/skywalking/pull/9914#discussion_r1015167279
##########
docs/en/changes/changes.md:
##########
@@ -97,6 +97,8 @@
* Support different indexType
* Support configuration for TTL and (block|segment) intervals
* Optimize MQ Topology analysis. Use entry span's peer from the consumer side as source service when no producer instrumentation(no cross-process reference).
+* Elasticsearch storage: Provide system environment variable(`SW_STORAGE_ES_SPECIFIC_INDEX_SETTINGS`) and support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
+* Elasticsearch storage: Support update index settings `(number_of_shards/number_of_replicas)` for the index template after re-configured.
Review Comment:
```suggestion
* Elasticsearch storage: Support update index settings `(number_of_shards/number_of_replicas)` for the index template after rebooting.
```
I think we need to make this and doc consistent. The doc doesn't mention rebooting and override existing index settings.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng merged pull request #9914: Elasticsearch storage: support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
Posted by GitBox <gi...@apache.org>.
wu-sheng merged PR #9914:
URL: https://github.com/apache/skywalking/pull/9914
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9914: Elasticsearch storage: support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9914:
URL: https://github.com/apache/skywalking/pull/9914#discussion_r1015161294
##########
docs/en/setup/backend/backend-storage.md:
##########
@@ -163,6 +166,37 @@ storage:
advanced: ${SW_STORAGE_ES_ADVANCED:"{\"index.translog.durability\":\"request\",\"index.translog.sync_interval\":\"5s\"}"}
```
+### Specify Settings For Each Elasticsearch Index Individually
Review Comment:
So, we support other configurations too? Besides shards and replications. Because the original #9849 mentioned these two configurations only.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wankai123 commented on a diff in pull request #9914: Elasticsearch storage: support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
Posted by GitBox <gi...@apache.org>.
wankai123 commented on code in PR #9914:
URL: https://github.com/apache/skywalking/pull/9914#discussion_r1015165881
##########
docs/en/setup/backend/backend-storage.md:
##########
@@ -163,6 +166,37 @@ storage:
advanced: ${SW_STORAGE_ES_ADVANCED:"{\"index.translog.durability\":\"request\",\"index.translog.sync_interval\":\"5s\"}"}
```
+### Specify Settings For Each Elasticsearch Index Individually
Review Comment:
We can support more if further required, I have listed the supported setting in the doc, for now only `number_of_shards/number_of_replicas`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] toffentoffen commented on a diff in pull request #9914: Elasticsearch storage: support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
Posted by GitBox <gi...@apache.org>.
toffentoffen commented on code in PR #9914:
URL: https://github.com/apache/skywalking/pull/9914#discussion_r1015963081
##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/StorageEsInstaller.java:
##########
@@ -111,28 +123,36 @@ private void createNormalTable(Model model) throws StorageException {
ElasticSearchClient esClient = (ElasticSearchClient) client;
String tableName = IndexController.INSTANCE.getTableName(model);
Mappings mapping = createMapping(model);
+ Map<String, Object> settings = createSetting(model);
if (!esClient.isExistsIndex(tableName)) {
- Map<String, Object> settings = createSetting(model);
boolean isAcknowledged = esClient.createIndex(tableName, mapping, settings);
log.info("create {} index finished, isAcknowledged: {}", tableName, isAcknowledged);
if (!isAcknowledged) {
- throw new StorageException("create " + tableName + " index failure, ");
+ throw new StorageException("create " + tableName + " index failure");
}
} else {
Mappings historyMapping = esClient.getIndex(tableName)
.map(Index::getMappings)
.orElseGet(Mappings::new);
- structures.putStructure(tableName, mapping);
- Mappings appendMapping = structures.diffStructure(tableName, historyMapping);
+ structures.putStructure(tableName, mapping, settings);
+ Mappings appendMapping = structures.diffMappings(tableName, historyMapping);
+ //update mapping
if (appendMapping.getProperties() != null && !appendMapping.getProperties().isEmpty()) {
boolean isAcknowledged = esClient.updateIndexMapping(tableName, appendMapping);
- log.info("update {} index finished, isAcknowledged: {}, append mappings: {}", tableName,
+ log.info("update {} index mapping finished, isAcknowledged: {}, append mapping: {}", tableName,
isAcknowledged, appendMapping
);
if (!isAcknowledged) {
- throw new StorageException("update " + tableName + " index failure");
+ throw new StorageException("update " + tableName + " index mapping failure");
}
}
+ //needs to update settings
+ if (!structures.compareIndexSetting(tableName, settings)) {
+ log.warn(
+ "index {} settings configuration has been updated to {}, please remove it before OAP starts",
+ tableName, settings
+ );
+ }
Review Comment:
Under which scenario, which type of setting, causes that the index needs to be removed? I guess that if specific number of shards or replicas are specified for a concrete index, this warning will be displayed isn't it?
I not sure if we should display it when only shards or replicas have changed:
1. User could be confused, as the shards and replica changes only apply for new indices after the changes are applied to the index template as per https://github.com/apache/skywalking/pull/9914/files#diff-7c42c99e33896647d67385263076ec5178df8e57d072e760895ed7ca9f5d1c20R199
2. Maybe we should display an info message instead if only shards and replica number have changed, as those changes can never be applied because ES doesn't allow to change the number of shards and replicas for and existing index. Moreover, I think those changes are never applied to an existing index, that's the reason why I would suggest to not show a warning message.
##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/IndexStructures.java:
##########
@@ -146,4 +172,18 @@ private Map<String, Object> diffFields(Fields fields) {
));
}
}
+
+ /**
+ * The index settings structure which only include needs to compare and update fields
+ */
+ @EqualsAndHashCode
+ public static class UpdatableIndexSettings {
+ private final int replicas;
+ private final int shards;
+
+ public UpdatableIndexSettings(Map<String, Object> indexSettings) {
+ this.replicas = Integer.parseInt((String) indexSettings.getOrDefault("number_of_replicas", "0"));
+ this.shards = Integer.parseInt((String) indexSettings.getOrDefault("number_of_shards", "0"));
Review Comment:
I am fully aware how it works globally, but I have some doubts.
1. Is it mandatory to set always both properties for a given index in the json to specify the concrete number of replicas and shards?
2. If only replicas is provided for instance, what will be the number of shards 0? Or the value specified by `SW_STORAGE_ES_INDEX_SHARDS_NUMBER` if provided or the default value of `SW_STORAGE_ES_INDEX_SHARDS_NUMBER` if absent?
##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/StorageEsInstaller.java:
##########
@@ -183,13 +214,14 @@ private void createTimeSeriesTable(Model model) throws StorageException {
protected Map<String, Object> createSetting(Model model) throws StorageException {
Map<String, Object> setting = new HashMap<>();
-
- setting.put("index.number_of_replicas", model.isSuperDataset()
- ? config.getSuperDatasetIndexReplicasNumber()
- : config.getIndexReplicasNumber());
- setting.put("index.number_of_shards", model.isSuperDataset()
- ? config.getIndexShardsNumber() * config.getSuperDatasetIndexShardsFactor()
- : config.getIndexShardsNumber());
+ Map<String, Object> indexSettings = new HashMap<>();
+ setting.put("index", indexSettings);
+ indexSettings.put("number_of_replicas", model.isSuperDataset()
+ ? Integer.toString(config.getSuperDatasetIndexReplicasNumber())
+ : Integer.toString(config.getIndexReplicasNumber()));
+ indexSettings.put("number_of_shards", model.isSuperDataset()
+ ? Integer.toString(config.getIndexShardsNumber() * config.getSuperDatasetIndexShardsFactor())
Review Comment:
I would suggest that we should not multiply the number of shards by the shards factor if a concrete number of shards is configure for a given index. Otherwise, the user might be confused. If the user says I want X number of shards for zipkin index, X number of shards should be used. Otherwise, when the users checks the number of shards it will see X*Shards_Factor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wankai123 commented on a diff in pull request #9914: Elasticsearch storage: support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
Posted by GitBox <gi...@apache.org>.
wankai123 commented on code in PR #9914:
URL: https://github.com/apache/skywalking/pull/9914#discussion_r1016092721
##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/IndexStructures.java:
##########
@@ -146,4 +172,18 @@ private Map<String, Object> diffFields(Fields fields) {
));
}
}
+
+ /**
+ * The index settings structure which only include needs to compare and update fields
+ */
+ @EqualsAndHashCode
+ public static class UpdatableIndexSettings {
+ private final int replicas;
+ private final int shards;
+
+ public UpdatableIndexSettings(Map<String, Object> indexSettings) {
+ this.replicas = Integer.parseInt((String) indexSettings.getOrDefault("number_of_replicas", "0"));
+ this.shards = Integer.parseInt((String) indexSettings.getOrDefault("number_of_shards", "0"));
Review Comment:
> 1. Is it mandatory to set always both properties for a given index in the json to specify the concrete number of replicas and shards?
No, we can set one or both, the absent one will use the generic settings.
see: https://skywalking.apache.org/docs/main/next/en/setup/backend/backend-storage/#index-settings
> 2. If only replicas is provided for instance, what will be the number of shards 0?
No, `UpdatableIndexSettings` structure only used for comparing old and new settings, so the the number of shards will be the the generic settings which I mentioned above.
If
```java
Integer.parseInt((String) indexSettings.getOrDefault("number_of_replicas", "0"));
```
cause some confusion, I'll submit a PR, and change it to
```java
String.valueOf(indexSettings.getOrDefault("number_of_replicas", Const.EMPTY_STRING));
```
>Or the value specified by `SW_STORAGE_ES_INDEX_SHARDS_NUMBER` if provided or the default value of `SW_STORAGE_ES_INDEX_SHARDS_NUMBER` if absent?
yes, they have a default value
see: https://github.com/apache/skywalking/blob/48a0985345bc65a63b17d4539f80de334383dedb/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/StorageModuleElasticsearchConfig.java#L60
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9914: Elasticsearch storage: support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9914:
URL: https://github.com/apache/skywalking/pull/9914#discussion_r1015166413
##########
docs/en/setup/backend/backend-storage.md:
##########
@@ -163,6 +166,37 @@ storage:
advanced: ${SW_STORAGE_ES_ADVANCED:"{\"index.translog.durability\":\"request\",\"index.translog.sync_interval\":\"5s\"}"}
```
+### Specify Settings For Each Elasticsearch Index Individually
+You can specify the settings for one or more indexes individually by using `SW_STORAGE_ES_SPECIFIC_INDEX_SETTINGS`.
+**NOTE:** If configured, this setting has the highest priority and overrides the generic settings.
+
+The settings in `JSON` format and the index name in the config should exclude the `${SW_NAMESPACE}` e.g.
Review Comment:
```suggestion
The settings in `JSON` format and the index name in the config should exclude the `storage/elasticsearch/ namespace` e.g.
```
SW_NAMESPACE system variable is not going to be overridden. We override things through kernel level.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9914: Elasticsearch storage: support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9914:
URL: https://github.com/apache/skywalking/pull/9914#discussion_r1015993463
##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/StorageEsInstaller.java:
##########
@@ -183,13 +214,14 @@ private void createTimeSeriesTable(Model model) throws StorageException {
protected Map<String, Object> createSetting(Model model) throws StorageException {
Map<String, Object> setting = new HashMap<>();
-
- setting.put("index.number_of_replicas", model.isSuperDataset()
- ? config.getSuperDatasetIndexReplicasNumber()
- : config.getIndexReplicasNumber());
- setting.put("index.number_of_shards", model.isSuperDataset()
- ? config.getIndexShardsNumber() * config.getSuperDatasetIndexShardsFactor()
- : config.getIndexShardsNumber());
+ Map<String, Object> indexSettings = new HashMap<>();
+ setting.put("index", indexSettings);
+ indexSettings.put("number_of_replicas", model.isSuperDataset()
+ ? Integer.toString(config.getSuperDatasetIndexReplicasNumber())
+ : Integer.toString(config.getIndexReplicasNumber()));
+ indexSettings.put("number_of_shards", model.isSuperDataset()
+ ? Integer.toString(config.getIndexShardsNumber() * config.getSuperDatasetIndexShardsFactor())
Review Comment:
Factor should only be used in default settings, not a part of override mechanism.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wankai123 commented on a diff in pull request #9914: Elasticsearch storage: support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
Posted by GitBox <gi...@apache.org>.
wankai123 commented on code in PR #9914:
URL: https://github.com/apache/skywalking/pull/9914#discussion_r1016013408
##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/StorageEsInstaller.java:
##########
@@ -111,28 +123,36 @@ private void createNormalTable(Model model) throws StorageException {
ElasticSearchClient esClient = (ElasticSearchClient) client;
String tableName = IndexController.INSTANCE.getTableName(model);
Mappings mapping = createMapping(model);
+ Map<String, Object> settings = createSetting(model);
if (!esClient.isExistsIndex(tableName)) {
- Map<String, Object> settings = createSetting(model);
boolean isAcknowledged = esClient.createIndex(tableName, mapping, settings);
log.info("create {} index finished, isAcknowledged: {}", tableName, isAcknowledged);
if (!isAcknowledged) {
- throw new StorageException("create " + tableName + " index failure, ");
+ throw new StorageException("create " + tableName + " index failure");
}
} else {
Mappings historyMapping = esClient.getIndex(tableName)
.map(Index::getMappings)
.orElseGet(Mappings::new);
- structures.putStructure(tableName, mapping);
- Mappings appendMapping = structures.diffStructure(tableName, historyMapping);
+ structures.putStructure(tableName, mapping, settings);
+ Mappings appendMapping = structures.diffMappings(tableName, historyMapping);
+ //update mapping
if (appendMapping.getProperties() != null && !appendMapping.getProperties().isEmpty()) {
boolean isAcknowledged = esClient.updateIndexMapping(tableName, appendMapping);
- log.info("update {} index finished, isAcknowledged: {}, append mappings: {}", tableName,
+ log.info("update {} index mapping finished, isAcknowledged: {}, append mapping: {}", tableName,
isAcknowledged, appendMapping
);
if (!isAcknowledged) {
- throw new StorageException("update " + tableName + " index failure");
+ throw new StorageException("update " + tableName + " index mapping failure");
}
}
+ //needs to update settings
+ if (!structures.compareIndexSetting(tableName, settings)) {
+ log.warn(
+ "index {} settings configuration has been updated to {}, please remove it before OAP starts",
+ tableName, settings
+ );
+ }
Review Comment:
Hi, this warning is only for the `Normal index` which would not generate new indexes with time series by index template. So the new settings will not be applied until the index is recreated. Such as index `sw_ui_template`
For the `Time series index` I log the info:
https://github.com/apache/skywalking/blob/48a0985345bc65a63b17d4539f80de334383dedb/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/StorageEsInstaller.java#L199
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wankai123 commented on a diff in pull request #9914: Elasticsearch storage: support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
Posted by GitBox <gi...@apache.org>.
wankai123 commented on code in PR #9914:
URL: https://github.com/apache/skywalking/pull/9914#discussion_r1016013383
##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/IndexStructures.java:
##########
@@ -146,4 +172,18 @@ private Map<String, Object> diffFields(Fields fields) {
));
}
}
+
+ /**
+ * The index settings structure which only include needs to compare and update fields
+ */
+ @EqualsAndHashCode
+ public static class UpdatableIndexSettings {
+ private final int replicas;
+ private final int shards;
+
+ public UpdatableIndexSettings(Map<String, Object> indexSettings) {
+ this.replicas = Integer.parseInt((String) indexSettings.getOrDefault("number_of_replicas", "0"));
+ this.shards = Integer.parseInt((String) indexSettings.getOrDefault("number_of_shards", "0"));
Review Comment:
Thanks, I'll check.
##########
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/StorageEsInstaller.java:
##########
@@ -111,28 +123,36 @@ private void createNormalTable(Model model) throws StorageException {
ElasticSearchClient esClient = (ElasticSearchClient) client;
String tableName = IndexController.INSTANCE.getTableName(model);
Mappings mapping = createMapping(model);
+ Map<String, Object> settings = createSetting(model);
if (!esClient.isExistsIndex(tableName)) {
- Map<String, Object> settings = createSetting(model);
boolean isAcknowledged = esClient.createIndex(tableName, mapping, settings);
log.info("create {} index finished, isAcknowledged: {}", tableName, isAcknowledged);
if (!isAcknowledged) {
- throw new StorageException("create " + tableName + " index failure, ");
+ throw new StorageException("create " + tableName + " index failure");
}
} else {
Mappings historyMapping = esClient.getIndex(tableName)
.map(Index::getMappings)
.orElseGet(Mappings::new);
- structures.putStructure(tableName, mapping);
- Mappings appendMapping = structures.diffStructure(tableName, historyMapping);
+ structures.putStructure(tableName, mapping, settings);
+ Mappings appendMapping = structures.diffMappings(tableName, historyMapping);
+ //update mapping
if (appendMapping.getProperties() != null && !appendMapping.getProperties().isEmpty()) {
boolean isAcknowledged = esClient.updateIndexMapping(tableName, appendMapping);
- log.info("update {} index finished, isAcknowledged: {}, append mappings: {}", tableName,
+ log.info("update {} index mapping finished, isAcknowledged: {}, append mapping: {}", tableName,
isAcknowledged, appendMapping
);
if (!isAcknowledged) {
- throw new StorageException("update " + tableName + " index failure");
+ throw new StorageException("update " + tableName + " index mapping failure");
}
}
+ //needs to update settings
+ if (!structures.compareIndexSetting(tableName, settings)) {
+ log.warn(
+ "index {} settings configuration has been updated to {}, please remove it before OAP starts",
+ tableName, settings
+ );
+ }
Review Comment:
Hi, this warning is only for the `Normal index` which would not generate new indexes with time series by index template. So the new settings will not be applied until the index is recreated.
For the `Time series index` I log the info:
https://github.com/apache/skywalking/blob/48a0985345bc65a63b17d4539f80de334383dedb/oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/StorageEsInstaller.java#L199
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9914: Elasticsearch storage: support specify the settings `(number_of_shards/number_of_replicas)` for each index individually.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9914:
URL: https://github.com/apache/skywalking/pull/9914#discussion_r1015294177
##########
docs/en/setup/backend/backend-storage.md:
##########
@@ -163,6 +180,39 @@ storage:
advanced: ${SW_STORAGE_ES_ADVANCED:"{\"index.translog.durability\":\"request\",\"index.translog.sync_interval\":\"5s\"}"}
```
+#### Specify Settings For Each Elasticsearch Index Individually
+You can specify the settings for one or more indexes individually by using `SW_STORAGE_ES_SPECIFIC_INDEX_SETTINGS`.
+
+**NOTE:**
+Supported settings:
+- number_of_shards
+- number_of_replicas
+
+**NOTE:** If configured, the settings in this configuration have the highest priority and will override the existing
+generic settings which configured by the others configuration items.
Review Comment:
```suggestion
**NOTE:** These settings have the highest priority and will override the existing
generic settings mentioned in [index settings doc](#index-settings).
```
Please check whether the new link is correct.
##########
docs/en/setup/backend/backend-storage.md:
##########
@@ -163,6 +180,39 @@ storage:
advanced: ${SW_STORAGE_ES_ADVANCED:"{\"index.translog.durability\":\"request\",\"index.translog.sync_interval\":\"5s\"}"}
```
+#### Specify Settings For Each Elasticsearch Index Individually
+You can specify the settings for one or more indexes individually by using `SW_STORAGE_ES_SPECIFIC_INDEX_SETTINGS`.
+
+**NOTE:**
+Supported settings:
+- number_of_shards
+- number_of_replicas
+
+**NOTE:** If configured, the settings in this configuration have the highest priority and will override the existing
+generic settings which configured by the others configuration items.
+
+The settings in `JSON` format and the index name in the config should exclude the `${SW_NAMESPACE}` which is `sw` by default, e.g.
Review Comment:
```suggestion
The settings are in `JSON` format. The index name here is logic entity name, which should exclude the `${SW_NAMESPACE}` which is `sw` by default, e.g.
```
##########
docs/en/setup/backend/backend-storage.md:
##########
@@ -151,7 +154,21 @@ Once it is changed manually or through a 3rd party tool, such as [Vault](https:/
the storage provider will use the new username, password, and JKS password to establish the connection and close the old one. If the information exists in the file,
the `user/password` will be overridden.
-### Advanced Configurations For Elasticsearch Index
+
+### Each Elasticsearch Index Settings
+You can set and update the index settings `(number_of_shards/number_of_replicas)` by the following configuration items for the index template after rebooting:
+```yaml
+storage:
+ elasticsearch:
+ # ......
+ indexShardsNumber: ${SW_STORAGE_ES_INDEX_SHARDS_NUMBER:1}
+ indexReplicasNumber: ${SW_STORAGE_ES_INDEX_REPLICAS_NUMBER:1}
+ specificIndexSettings: ${SW_STORAGE_ES_SPECIFIC_INDEX_SETTINGS:""}
+ superDatasetIndexShardsFactor: ${SW_STORAGE_ES_SUPER_DATASET_INDEX_SHARDS_FACTOR:5}
+ superDatasetIndexReplicasNumber: ${SW_STORAGE_ES_SUPER_DATASET_INDEX_REPLICAS_NUMBER:0}
Review Comment:
I think as we use merged index policy by default, we could document which indicates affected by which configurations clearly.
##########
docs/en/setup/backend/backend-storage.md:
##########
@@ -163,6 +180,39 @@ storage:
advanced: ${SW_STORAGE_ES_ADVANCED:"{\"index.translog.durability\":\"request\",\"index.translog.sync_interval\":\"5s\"}"}
```
+#### Specify Settings For Each Elasticsearch Index Individually
+You can specify the settings for one or more indexes individually by using `SW_STORAGE_ES_SPECIFIC_INDEX_SETTINGS`.
+
+**NOTE:**
+Supported settings:
+- number_of_shards
+- number_of_replicas
+
+**NOTE:** If configured, the settings in this configuration have the highest priority and will override the existing
+generic settings which configured by the others configuration items.
+
+The settings in `JSON` format and the index name in the config should exclude the `${SW_NAMESPACE}` which is `sw` by default, e.g.
+```json
+{
+ "metrics-all":{
+ "number_of_shards":"3",
+ "number_of_replicas":"2"
+ },
+ "segment":{
+ "number_of_shards":"6",
+ "number_of_replicas":"1"
+ }
+}
+```
+
+And put it into the following configure item:
Review Comment:
```suggestion
This configuration in the YAML file is like this,
```
##########
docs/en/setup/backend/backend-storage.md:
##########
@@ -151,7 +154,21 @@ Once it is changed manually or through a 3rd party tool, such as [Vault](https:/
the storage provider will use the new username, password, and JKS password to establish the connection and close the old one. If the information exists in the file,
the `user/password` will be overridden.
-### Advanced Configurations For Elasticsearch Index
+
+### Each Elasticsearch Index Settings
+You can set and update the index settings `(number_of_shards/number_of_replicas)` by the following configuration items for the index template after rebooting:
Review Comment:
```suggestion
### Index Settings
The following settings control the number of shards and replicas for new and existing index templates. The update only got applied after OAP reboots.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org