feat(elasticsearch): Implement optimization to use reindexing instead… #8352

iprentic · 2023-06-30T15:06:50Z

… of deleteByQuery when a large amount of records will be deleted

Checklist

The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
Links to related issues (if applicable)
Tests for the changes have been added/updated (if applicable)
Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

david-leifker · 2023-06-30T21:45:25Z

metadata-io/src/main/java/com/linkedin/metadata/graph/elastic/ElasticSearchGraphService.java

@@ -326,6 +328,12 @@ public void configure() {
    }
  }

+  @Override
+  public String reindexAsync(String index, @Nullable QueryBuilder filterQuery, BatchWriteOperationsOptions options)


At first glance, this doesn't seem right to me. The graph service index is not timeseries and would not be subject to timeseries-like truncation. Remove reindexAsync from the common interface. It is only needed on the time series related classes.

david-leifker · 2023-06-30T21:48:46Z

metadata-io/src/main/java/com/linkedin/metadata/search/elasticsearch/ElasticSearchService.java

@@ -44,6 +46,12 @@ public void configure() {
    indexBuilders.reindexAll();
  }

+  @Override
+  public String reindexAsync(String index, @Nullable QueryBuilder filterQuery, BatchWriteOperationsOptions options)


Same as above, remove from the interface (also applies to below) non-timeseries Service classes.

iprentic · 2023-06-30T21:55:28Z

...io/src/main/java/com/linkedin/metadata/search/elasticsearch/indexbuilder/ESIndexBuilder.java

@@ -220,6 +224,27 @@ public void buildIndex(ReindexConfig indexState) throws IOException {
    }
  }

+  public String reindexInPlaceAsync(String indexAlias, @Nullable QueryBuilder filterQuery, BatchWriteOperationsOptions options)


Can do, I'll move that. That seems better than taking in the batch size and timeout as parameters since we may want to do other tuning (or support other storages that have different parameters) in the future

david-leifker · 2023-06-30T22:01:02Z

...io/src/main/java/com/linkedin/metadata/search/elasticsearch/indexbuilder/ESIndexBuilder.java

@@ -220,6 +224,27 @@ public void buildIndex(ReindexConfig indexState) throws IOException {
    }
  }

+  public String reindexInPlaceAsync(String indexAlias, @Nullable QueryBuilder filterQuery, BatchWriteOperationsOptions options)


This is very much just for timeseries, I'd say we should name it differently to make that clear. It also then makes since that a timeseries method would take a timeseries package options parameter.

david-leifker · 2023-06-30T22:02:15Z

metadata-io/src/main/java/com/linkedin/metadata/shared/ElasticSearchIndexed.java



 public interface ElasticSearchIndexed {
+    String reindexAsync(String index, @Nullable QueryBuilder filterQuery, BatchWriteOperationsOptions options)


Lets remove it from this interface, it is not generally a method for all indices at this time.

david-leifker · 2023-06-30T22:08:13Z

...ava/com/linkedin/metadata/timeseries/elastic/indexbuilder/TimeseriesAspectIndexBuilders.java

@@ -33,6 +36,12 @@ public void reindexAll() {
    }
  }

+  @Override


Should exist, perhaps not an override but depending on how the clean-up of the ElasticsearchIndex interface goes, you may end up wanting to create a new interface like ElasticSearchIndexedTimeseries (yikes that's long). The timeseries interface applies to timeseries Services' indices (or a subset). Most Services will not be implementing the tiimeseries methods: entity, system metadata, and graph for example do not require the timeseries truncations.

iprentic · 2023-07-07T20:08:35Z

Testing on quickstart instance:

Dry run:

$ curl --location --request POST  http://localhost:8080/operations\?action\=truncateTimeseriesAspect \
--header 'Content-Type: application/json' \
--data-raw '{
    "entityType": "dataset",
    "aspect": "datasetusagestatistics",
    "endTimeMillis": 1687970192000,
    "dryRun": true,
    "batchSize": 100,
    "timeoutSeconds": 3600
}'
{"value":"Delete 134517 out of 135600 rows (99.20%). Reindexing the aspect without the deleted records. This was a dry run. Run with dryRun = false to execute."}%

Execute:

$ curl --location --request POST  http://localhost:8080/operations\?action\=truncateTimeseriesAspect \
--header 'Content-Type: application/json' \
--data-raw '{
    "entityType": "dataset",
    "aspect": "datasetusagestatistics",
    "endTimeMillis": 1687970192000,
    "dryRun": false,
    "batchSize": 100,
    "timeoutSeconds": 3600
}'
{"value":"qhxGdzytQS-pQek8CwBCZg:533446"}%

Number of records after the reindex:

$ curl -X GET "localhost:9200/dataset_datasetusagestatisticsaspect_v1/_count?pretty"                  
{
  "count" : 138,
  "_shards" : {
    "total" : 1,
    "successful" : 1,
    "skipped" : 0,
    "failed" : 0
  }
}

… of deleteByQuery when a large amount of records will be deleted

github-actions bot added product PR or Issue related to the DataHub UI/UX devops PR or Issue related to DataHub backend & deployment labels Jun 30, 2023

david-leifker reviewed Jun 30, 2023

View reviewed changes

iprentic commented Jun 30, 2023

View reviewed changes

david-leifker reviewed Jun 30, 2023

View reviewed changes

vercel bot deployed to Preview June 30, 2023 23:22 View deployment

vercel bot had a problem deploying to Preview July 5, 2023 22:46 Failure

iprentic changed the title ~~[WIP] feat(elasticsearch): Implement optimization to use reindexing instead…~~ feat(elasticsearch): Implement optimization to use reindexing instead… Jul 7, 2023

vercel bot had a problem deploying to Preview July 7, 2023 20:15 Failure

Indy Prentice added 6 commits July 10, 2023 14:34

feat(elasticsearch): Implement optimization to use reindexing instead…

3944827

… of deleteByQuery when a large amount of records will be deleted

Move method out of interface, add one test

7df3ae5

Create new index with the correct mappings first

84a0c31

update error message

e0d4ffb

bugfix

141470c

Do not delete the old alias

2f1cfef

iprentic force-pushed the nd-reindex branch from bc352a5 to 2f1cfef Compare July 10, 2023 19:34

vercel bot deployed to Preview July 10, 2023 20:22 View deployment

david-leifker self-requested a review July 11, 2023 16:39

iprentic requested a review from RyanHolstien July 11, 2023 18:27

david-leifker approved these changes Jul 11, 2023

View reviewed changes

anshbansal merged commit a884cf3 into master Jul 12, 2023
36 checks passed

anshbansal deleted the nd-reindex branch July 12, 2023 11:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(elasticsearch): Implement optimization to use reindexing instead… #8352

feat(elasticsearch): Implement optimization to use reindexing instead… #8352

iprentic commented Jun 30, 2023 •

edited

Loading

david-leifker Jun 30, 2023 •

edited

Loading

david-leifker Jun 30, 2023 •

edited

Loading

iprentic Jun 30, 2023

david-leifker Jun 30, 2023

david-leifker Jun 30, 2023

david-leifker Jun 30, 2023

iprentic commented Jul 7, 2023



		public interface ElasticSearchIndexed {
		String reindexAsync(String index, @Nullable QueryBuilder filterQuery, BatchWriteOperationsOptions options)

feat(elasticsearch): Implement optimization to use reindexing instead… #8352

feat(elasticsearch): Implement optimization to use reindexing instead… #8352

Conversation

iprentic commented Jun 30, 2023 • edited Loading

Checklist

david-leifker Jun 30, 2023 • edited Loading

Choose a reason for hiding this comment

david-leifker Jun 30, 2023 • edited Loading

Choose a reason for hiding this comment

iprentic Jun 30, 2023

Choose a reason for hiding this comment

david-leifker Jun 30, 2023

Choose a reason for hiding this comment

david-leifker Jun 30, 2023

Choose a reason for hiding this comment

david-leifker Jun 30, 2023

Choose a reason for hiding this comment

iprentic commented Jul 7, 2023

iprentic commented Jun 30, 2023 •

edited

Loading

david-leifker Jun 30, 2023 •

edited

Loading

david-leifker Jun 30, 2023 •

edited

Loading