Extract metadata log operations from FlintClient into FlintMetadataLogService #379

seankao-az · 2024-06-11T23:53:22Z

Description

Extract metadata log operations from FlintClient into FlintMetadataLogService.
~~Add flint options for custom class path and instantiate FlintMetadataLogService through reflection.~~ re-evaluate in another PR
FlintClient.getIndexMetadata no longer attaches metadata log entry, as it has no access to FlintMetadataLogService. This is moved to FlintSpark.describeIndex.
Abstract recordHeartbeat method
Collect metadata log related test cases into a new suite FlintMetadataLogITSuite

Issues Resolved

[FEATURE] Index metadata service refactor #371

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Sean Kao <seankao@amazon.com>

FlintClientBuilder uses reflection to instantiate FlintMetadataLogService, which cannot be checked at compile time. Signed-off-by: Sean Kao <seankao@amazon.com>

Signed-off-by: Sean Kao <seankao@amazon.com>

ykmr1224 · 2024-06-13T00:17:38Z

flint-core/src/main/scala/org/opensearch/flint/core/metadata/log/FlintMetadataLogService.java

+   * Start a new optimistic transaction.
+   *
+   * @param indexName index name
+   * @param forceInit forceInit create empty metadata log if not exist.


forceInit sounds more like it tries to init even metadata already exist. initIfNotExist or initWhenNeeded should be better naming.

changed to use initIfNotExist for getMetadataLog. Keeping it as forceInit for startTransaction, as the semantics I think is to force init the transaction, instead of init the metadata log

...core/src/main/scala/org/opensearch/flint/core/storage/FlintOpenSearchMetadataLogService.java

Signed-off-by: Sean Kao <seankao@amazon.com>

penghuo · 2024-06-13T17:47:54Z

flint-core/src/main/scala/org/opensearch/flint/core/storage/FlintOpenSearchClient.java


-  public FlintOpenSearchClient(FlintOptions options) {
+  public FlintOpenSearchClient(FlintOptions options, FlintMetadataLogService metadataLogService) {


We should better naming FlintOpenSearchClient, the reasons it FlintOpenSearchClient depend FlintMedataLogService, not OpenSearch. Or in the other way,

Remove FlintMedataLogService operation from FlintOpenSearchClient

Modify FlintOpenSearchClient caller to call FlintMedataLogService

+1 for above comment.
Also, what is the value add of FlintOpenSearchClient if we are passing down everything to FlintMetadataLogService?

Can we replace FlintOpenSearchClient with FlintMetadataLogService where ever we make operations on FlintMetadataLog.

For startTransaction I agree that we can make the callers use FlintMetadataLogService instead. However, FlintOpenSearchClient getIndexMetadata still needs FlintMetadataLogService to getMetadataLog.

Will try to move it away from getIndexMetadata and instead use it in its caller (FlintSpark describeIndex)

Previously we only initialize request index when starting transaction in createIndex. Now current change is to init whenever new API getIndexMetadataLog called? Not sure the impact and if benefit on code structure.

Now current change is to init whenever new API getIndexMetadataLog called?

Sure... init is indeed not required. Changed so that the FlintMetadataLogService public interface for getIndexMetadataLog cannot init request index. It gets metadata log if available.
Now the initIfNotExist is only used as private method for FlintOpenSearchMetadataLogService as part of startTransaction with forceInit.

Removed FlintMetadataLogService operation from FlintOpenSearchClient

flint-core/src/main/scala/org/opensearch/flint/core/metadata/log/FlintMetadataLogService.java

noCharger · 2024-06-14T06:35:52Z

Any plan to move FlintClient / FlintMetadataLogService to commons module?

Signed-off-by: Sean Kao <seankao@amazon.com>

seankao-az · 2024-06-14T16:25:23Z

move FlintMetadataLogService to commons module

will do in separate PR

Signed-off-by: Sean Kao <seankao@amazon.com>

penghuo · 2024-06-17T23:32:46Z

flint-spark-integration/src/main/scala/org/opensearch/flint/spark/FlintSpark.scala

+    val latest = flintMetadataLogService
+      .getIndexMetadataLog(indexName)
+      .flatMap(_.getLatest)
+    if (latest.isPresent) {


may be not relelated to this PR, why latest metadatalog is requried? seems it could be optional?

this was introduced as part of show flint index statement, to get index state. And indeed it is optional. The log entry is added to the FlintMetadata if available

dai-chen

Thanks for the changes!

…gService (opensearch-project#379) * metadata log service interface Signed-off-by: Sean Kao <seankao@amazon.com> * flint opensearch metadata log service impl Signed-off-by: Sean Kao <seankao@amazon.com> * reflection for metadata log service instantiate Signed-off-by: Sean Kao <seankao@amazon.com> * add comments Signed-off-by: Sean Kao <seankao@amazon.com> * undo using client builder in tests FlintClientBuilder uses reflection to instantiate FlintMetadataLogService, which cannot be checked at compile time. Signed-off-by: Sean Kao <seankao@amazon.com> * use metadata log service in transaction IT suites Signed-off-by: Sean Kao <seankao@amazon.com> * metadata log test suite Signed-off-by: Sean Kao <seankao@amazon.com> * flint client builder test Signed-off-by: Sean Kao <seankao@amazon.com> * scalafmtAll Signed-off-by: Sean Kao <seankao@amazon.com> * undo reflection Signed-off-by: Sean Kao <seankao@amazon.com> * rename descriptive symbols Signed-off-by: Sean Kao <seankao@amazon.com> * descriptive parameter name and update comment Signed-off-by: Sean Kao <seankao@amazon.com> * remove init from getMetadataLog interface Signed-off-by: Sean Kao <seankao@amazon.com> * remove startTransaction from FlintClient Signed-off-by: Sean Kao <seankao@amazon.com> * rm flint os client dep on metadata log service Signed-off-by: Sean Kao <seankao@amazon.com> * rename function and remove unused flint client Signed-off-by: Sean Kao <seankao@amazon.com> * remove test case for init in getMetadataLog Signed-off-by: Sean Kao <seankao@amazon.com> * abstract recordHeartbeat method Signed-off-by: Sean Kao <seankao@amazon.com> * consistent var naming for metadataLogIndexName Signed-off-by: Sean Kao <seankao@amazon.com> --------- Signed-off-by: Sean Kao <seankao@amazon.com>

seankao-az added 3 commits June 11, 2024 16:43

metadata log service interface

9faadf4

Signed-off-by: Sean Kao <seankao@amazon.com>

flint opensearch metadata log service impl

8b715ec

Signed-off-by: Sean Kao <seankao@amazon.com>

reflection for metadata log service instantiate

36c7df4

Signed-off-by: Sean Kao <seankao@amazon.com>

seankao-az self-assigned this Jun 11, 2024

seankao-az added maintenance Code refactoring 0.5 labels Jun 11, 2024

seankao-az added 5 commits June 12, 2024 08:55

add comments

54064a1

Signed-off-by: Sean Kao <seankao@amazon.com>

undo using client builder in tests

ddb1ac2

FlintClientBuilder uses reflection to instantiate FlintMetadataLogService, which cannot be checked at compile time. Signed-off-by: Sean Kao <seankao@amazon.com>

use metadata log service in transaction IT suites

d1de5e6

Signed-off-by: Sean Kao <seankao@amazon.com>

metadata log test suite

a1d6925

Signed-off-by: Sean Kao <seankao@amazon.com>

flint client builder test

4d8cba9

Signed-off-by: Sean Kao <seankao@amazon.com>

seankao-az force-pushed the metadata-log-service branch from af23a72 to 4d8cba9 Compare June 12, 2024 18:54

seankao-az marked this pull request as ready for review June 12, 2024 19:04

seankao-az requested review from dai-chen, rupal-bq, vamsimanohar, penghuo, anirudha, kaituo and YANG-DB as code owners June 12, 2024 19:04

seankao-az mentioned this pull request Jun 12, 2024

[FEATURE] Index metadata service refactor #371

Closed

seankao-az added 2 commits June 12, 2024 13:27

scalafmtAll

667f5c2

Signed-off-by: Sean Kao <seankao@amazon.com>

Merge branch 'main' into metadata-log-service

af971e7

ykmr1224 reviewed Jun 13, 2024

View reviewed changes

seankao-az marked this pull request as draft June 13, 2024 01:44

seankao-az added 2 commits June 12, 2024 18:47

undo reflection

f4e90e4

Signed-off-by: Sean Kao <seankao@amazon.com>

rename descriptive symbols

819f711

Signed-off-by: Sean Kao <seankao@amazon.com>

seankao-az marked this pull request as ready for review June 13, 2024 01:59

descriptive parameter name and update comment

6b86c11

Signed-off-by: Sean Kao <seankao@amazon.com>

penghuo reviewed Jun 13, 2024

View reviewed changes

vamsimanohar reviewed Jun 13, 2024

View reviewed changes

flint-core/src/main/scala/org/opensearch/flint/core/metadata/log/FlintMetadataLogService.java Outdated Show resolved Hide resolved

seankao-az added 2 commits June 14, 2024 08:41

remove init from getMetadataLog interface

f534a98

Signed-off-by: Sean Kao <seankao@amazon.com>

remove startTransaction from FlintClient

8bad367

Signed-off-by: Sean Kao <seankao@amazon.com>

rm flint os client dep on metadata log service

88de3a3

Signed-off-by: Sean Kao <seankao@amazon.com>

seankao-az force-pushed the metadata-log-service branch from d81a35d to 79c7937 Compare June 14, 2024 19:19

seankao-az added 2 commits June 14, 2024 12:25

rename function and remove unused flint client

7c4d0ec

Signed-off-by: Sean Kao <seankao@amazon.com>

remove test case for init in getMetadataLog

9664595

Signed-off-by: Sean Kao <seankao@amazon.com>

seankao-az force-pushed the metadata-log-service branch from 79c7937 to 9664595 Compare June 14, 2024 19:28

seankao-az requested review from penghuo, vamsimanohar and ykmr1224 June 14, 2024 19:32

abstract recordHeartbeat method

346e344

Signed-off-by: Sean Kao <seankao@amazon.com>

seankao-az force-pushed the metadata-log-service branch from 9502ad6 to 346e344 Compare June 14, 2024 22:16

consistent var naming for metadataLogIndexName

f612fa7

Signed-off-by: Sean Kao <seankao@amazon.com>

penghuo reviewed Jun 17, 2024

View reviewed changes

seankao-az requested a review from penghuo June 18, 2024 17:04

dai-chen approved these changes Jun 18, 2024

View reviewed changes

seankao-az merged commit d0caa7b into opensearch-project:main Jun 18, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract metadata log operations from FlintClient into FlintMetadataLogService #379

Extract metadata log operations from FlintClient into FlintMetadataLogService #379

seankao-az commented Jun 11, 2024 •

edited

Loading

ykmr1224 Jun 13, 2024

seankao-az Jun 13, 2024 •

edited

Loading

penghuo Jun 13, 2024

vamsimanohar Jun 13, 2024 •

edited

Loading

seankao-az Jun 13, 2024 •

edited

Loading

dai-chen Jun 13, 2024 •

edited

Loading

seankao-az Jun 14, 2024 •

edited

Loading

seankao-az Jun 14, 2024 •

edited

Loading

noCharger commented Jun 14, 2024

seankao-az commented Jun 14, 2024

penghuo Jun 17, 2024

seankao-az Jun 18, 2024 •

edited

Loading

dai-chen left a comment


		public FlintOpenSearchClient(FlintOptions options) {
		public FlintOpenSearchClient(FlintOptions options, FlintMetadataLogService metadataLogService) {

Extract metadata log operations from FlintClient into FlintMetadataLogService #379

Extract metadata log operations from FlintClient into FlintMetadataLogService #379

Conversation

seankao-az commented Jun 11, 2024 • edited Loading

Description

Issues Resolved

ykmr1224 Jun 13, 2024

Choose a reason for hiding this comment

seankao-az Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

penghuo Jun 13, 2024

Choose a reason for hiding this comment

vamsimanohar Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

seankao-az Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

dai-chen Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

seankao-az Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

seankao-az Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

noCharger commented Jun 14, 2024

seankao-az commented Jun 14, 2024

penghuo Jun 17, 2024

Choose a reason for hiding this comment

seankao-az Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

dai-chen left a comment

Choose a reason for hiding this comment

seankao-az commented Jun 11, 2024 •

edited

Loading

seankao-az Jun 13, 2024 •

edited

Loading

vamsimanohar Jun 13, 2024 •

edited

Loading

seankao-az Jun 13, 2024 •

edited

Loading

dai-chen Jun 13, 2024 •

edited

Loading

seankao-az Jun 14, 2024 •

edited

Loading

seankao-az Jun 14, 2024 •

edited

Loading

seankao-az Jun 18, 2024 •

edited

Loading