[#122] feat(core): Add KV backend storage interface for EntityStore #123

yuqi1129 · 2023-07-25T12:27:13Z

What changes were proposed in this pull request?

This PR only add the KV backend interface for EntityStore

Why are the changes needed?

Currently, We only support storing entity in memory and can't keep it in persistent storage which will be used latter

Fix: #122

Does this PR introduce any user-facing change?

No

How was this patch tested?

Not yet

…style problem

core/src/main/java/com/datastrato/graviton/proto/CatalogEntitySerDe.java

jerryshao · 2023-07-26T01:56:10Z

storage/src/main/java/com/datastrato/graviton/storage/kv/KvEntityStore.java

+
+  @Override
+  public <R> R executeInTransaction(Executable<R> executable) throws IOException {
+    lock.lock();


Lock is only workable for embedded storage, maybe we should change to transactions.

Yes, I have add this as a TODO item.

jerryshao · 2023-07-26T02:16:31Z

Seems like you've moved the interface to storage module. I'm wondering how will you manage the code, will you also put rocksdb implementation in the storage module, if so, how will you manage the code if you have different storage implementations?

I was thinking of putting interfaces in core module storage package. And the rocksdb implementation in new storage-rocksdb module. With this, we could create a new module storage-xxx when we have new implementations.

### What changes were proposed in this pull request? This PR rethrow the real exception when function fails in transaction ### Why are the changes needed? Transaction caller needs to handle the real exception Fix: #165 ### Does this PR introduce _any_ user-facing change? Transaction caller should handle the exception which generated by `Executable` ### How was this patch tested? UT modified.

### What changes were proposed in this pull request? Add diqiu50 to maintainer ### Why are the changes needed? Fix: #755 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? No

### What changes were proposed in this pull request? (Please outline the changes and how this PR fixes the issue.) ### Why are the changes needed? (Please clarify why the changes are needed. For instance, 1. If you propose a new API, clarify the use case for a new API. 2. If you fix a bug, describe the bug.) Fix: # (issue) ### Does this PR introduce _any_ user-facing change? (Please list the user-facing changes introduced by your change, including 1. Change in user-facing APIs. 2. Addition or removal of property keys.) ### How was this patch tested? (Please test your changes, and provide instructions on how to test it: 1. If you add a feature or fix a bug, add a test to cover your changes. 2. If you fix a flaky test, repeat it for many times to prove it works.)

)  ### What changes were proposed in this pull request? close #1724 ### Why are the changes needed? Add server-side REST API support for fileset Fix: #1724 ### Does this PR introduce _any_ user-facing change? (Please list the user-facing changes introduced by your change, including 1. Change in user-facing APIs. 2. Addition or removal of property keys.) ### How was this patch tested? - add unit tests

…2043)  ### What changes were proposed in this pull request? Unify the style of configuration options ### Why are the changes needed? Fix: #2042 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Just doc. Co-authored-by: Heng Qin <qqtt@123.com>

…2045)  ### What changes were proposed in this pull request? Unify the style of configuration options ### Why are the changes needed? Fix: #2042 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Just doc. Co-authored-by: qqqttt123 <148952220+qqqttt123@users.noreply.github.com> Co-authored-by: Heng Qin <qqtt@123.com>

…uri (#2136)  ### What changes were proposed in this pull request? Change `gravitino.url` to `gravitino.uri`. ### Why are the changes needed? The example in trino-connector/installation.md uses different keys for gravitino link. https://github.com/datastrato/gravitino/blob/edfcdbea89dbf16d8e48833573c8917f9bcd5972/docs/trino-connector/installation.md?plain=1#L75-L81 From source code, it should be `gravitino.uri`. https://github.com/datastrato/gravitino/blob/edfcdbea89dbf16d8e48833573c8917f9bcd5972/trino-connector/src/main/java/com/datastrato/gravitino/trino/connector/GravitinoConfig.java#L22 We also need to fix the one in trino-connector/configuration.md. https://github.com/datastrato/gravitino/blob/edfcdbea89dbf16d8e48833573c8917f9bcd5972/docs/trino-connector/configuration.md?plain=1#L13 Fix: #2118 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Check there is no `gravitino.url` in the document. Signed-off-by: PoAn Yang <payang@apache.org>

…uri (#2142)  ### What changes were proposed in this pull request? Change `gravitino.url` to `gravitino.uri`. ### Why are the changes needed? The example in trino-connector/installation.md uses different keys for gravitino link. https://github.com/datastrato/gravitino/blob/edfcdbea89dbf16d8e48833573c8917f9bcd5972/docs/trino-connector/installation.md?plain=1#L75-L81 From source code, it should be `gravitino.uri`. https://github.com/datastrato/gravitino/blob/edfcdbea89dbf16d8e48833573c8917f9bcd5972/trino-connector/src/main/java/com/datastrato/gravitino/trino/connector/GravitinoConfig.java#L22 We also need to fix the one in trino-connector/configuration.md. https://github.com/datastrato/gravitino/blob/edfcdbea89dbf16d8e48833573c8917f9bcd5972/docs/trino-connector/configuration.md?plain=1#L13 Fix: #2118 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Check there is no `gravitino.url` in the document. Signed-off-by: PoAn Yang <payang@apache.org> Co-authored-by: PoAn Yang <payang@apache.org>

### What changes were proposed in this pull request? Dependencies follow alphabetical order ### Why are the changes needed? For better readability Fix: #738 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? N/A --------- Co-authored-by: Qi Yu <yuqi@datastrato.com>

### What changes were proposed in this pull request? Replace LinkedList with ArrayList. ### Why are the changes needed? Address linter issues mentioned in #2163. Fix #2163 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Covered by existing tests.

…#2138)  ### What changes were proposed in this pull request? close #1955 ### Why are the changes needed? (Please clarify why the changes are needed. For instance, 1. If you propose a new API, clarify the use case for a new API. 2. If you fix a bug, describe the bug.) Fix: #1955 ### Does this PR introduce _any_ user-facing change? - no ### How was this patch tested? - unit

…n atomic to keep thread safe (#2139)  ### What changes were proposed in this pull request? close #1953 #1946 #1941 #1920 #1910 ### Why are the changes needed? Fix: #1953 #1946 #1941 #1920 #1910 ### Does this PR introduce _any_ user-facing change? - no ### How was this patch tested? - ut

### What changes were proposed in this pull request? - close #2154 ### Why are the changes needed? - Fix: #2154 ### Does this PR introduce _any_ user-facing change? - no ### How was this patch tested? `./gradlew build` and UTs

…2403)  ### What changes were proposed in this pull request? - close #2367 ### Why are the changes needed? - Fix: #2367 ### Does this PR introduce _any_ user-facing change? - no ### How was this patch tested? - `./gradlew build` --------- Co-authored-by: mchades <liminghuang@datastrato.com>

…for consistency (#2437)  ### What changes were proposed in this pull request? Make all Spark SQL keywords used in spark connector integration test uppercase ### Why are the changes needed? Fix: #2424 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? `./gradlew build -PskipTests`

…le clean (#2531)  ### What changes were proposed in this pull request? Remove directory `spark-warehouse` when exectue gradle clean in moudle catalog-lakehouse-iceberg. Remove directory `src/main/resources` when execute gradle clean in module server, since which is generated by gradle build. ### Why are the changes needed? Fix: #2508 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? `./gradlew :catalogs:catalog-lakehouse-iceberg:clean` `./gradlew :server:clean` --------- Co-authored-by: Qi Yu <yuqi@datastrato.com>

…rdedByInstance error-prone (#2514)  ### What changes were proposed in this pull request? `StaticGuardedByInstance` amis to avoid `public static` fields. Because this field is not thread-safe. See more details at https://errorprone.info/bugpattern/StaticGuardedByInstance. In this MR, I remove some test code in `com.datastrato.gravitino.trino.connector.GravitinoPlugin` and `com.datastrato.gravitino.trino.connector.GravitinoConnectorFactory`. At the same time, I enable `StaticGuardedByInstance` error-prone. ### Why are the changes needed? - Fix: #2167 ### Does this PR introduce _any_ user-facing change? - no ### How was this patch tested? - original unit tests.

### What changes were proposed in this pull request? Add PoAn Yang (FrankYang0529) to Contributors list. Signed-off-by: PoAn Yang <payang@apache.org>

…ate_fileset function (#3535)  ### What changes were proposed in this pull request? * Change the suggest type of `type` from `Catalog.Type` to `Fileset.Type` ### Why are the changes needed? Fix: #3508 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? `./gradlew :clients:client-python:test` Co-authored-by: noidname01 <55401762+noidname01@users.noreply.github.com> Co-authored-by: TimWang <tim.wang@pranaq.com>

…tor (#2635)  ### What changes were proposed in this pull request? - support GravitinoCatalogStore to register the catalog. In the MR, we will support to create the hive catalog. ### Why are the changes needed? - Fix #3362 ### Does this PR introduce _any_ user-facing change? - support flink in gravitino ### How was this patch tested? - add UTs

… for client-python (#3568)  ### What changes were proposed in this pull request? * Replace varaible names `type` and `property`, which is Python functions and keywords, with more readable and custom variable names ### Why are the changes needed? Fix: #3561 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? `./gradlew clients:client-python:test` Co-authored-by: noidname01 <55401762+noidname01@users.noreply.github.com> Co-authored-by: TimWang <tim.wang@pranaq.com>

…3703)  ### What changes were proposed in this pull request? Add a Hive catalog property "list-all-tables". Using this property to control whether the Iceberg table is displayed in the Hive table list. ### Why are the changes needed? The bug is a schema has the Iceberg tables in the Hive catalog Fix: #3403 ### Does this PR introduce _any_ user-facing change? N/A. ### How was this patch tested? 1.create a hive catalog with "list-all-tables " property. 2.crate a database and a iceberg table in the catalog by hive beeline 3.check whether the table is displayed in the catalog . --------- Co-authored-by: ericqin <ericqin@tencent.com>

…c property issue (apache#3014)  ### What changes were proposed in this pull request? Load catalogs first when refresh schema or table level page <img width="1395" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/f3af84e6-d6dc-474e-9f55-e714edb9be35"> Fix kafka topic property issue ### Why are the changes needed? Fix: apache#2996, Fix: apache#3011 ### Does this PR introduce _any_ user-facing change? When schema, table or fileset page network error, refresh the page ### How was this patch tested? <img width="735" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/d69bc9f3-d4f7-4f1e-bc2c-65941821a510">

…hema level and kafka topic level (apache#2937) …a topic level  ### What changes were proposed in this pull request? Improvement of fileset and kafka test case ### Why are the changes needed? Fix: apache#2896 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? <img width="340" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/520204d9-2d5b-4050-b020-ad47275d2469">

…ve the interceptors of axios (apache#3116)  ### What changes were proposed in this pull request? 1. Support special character like '//' for database name or table name and so on. <img width="852" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/9ae20ce1-3683-453a-a651-53a2277a8e25"> 3. Improvment interceptors of axois for avoide repeat request 4. Fix some runtime errors ### Why are the changes needed? N/A Fix: apache#3062 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Manual

### What changes were proposed in this pull request? Fixed assertion Fix: apache#3251 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Ran test locally multiple times --------- Signed-off-by: Rohit Satya <work.rohitsatya@gmail.com>

…n null cause list tables error on some jdbc 8.x driver (apache#3294)  ### What changes were proposed in this pull request? replace `DatabaseMetaData#getSchema` by `DatabaseMetaData#getCatalog` and provide a more general way to get tables through jdbc driver ### Why are the changes needed? jdbc driver `DatabaseMetaData#getSchema` method return null causes list tables failure on some jdbc mysql 8.x driver (mysql-connector-java-8.0.11). Fix: apache#3269 ### Does this PR introduce _any_ user-facing change? not ### How was this patch tested? (Please test your changes, and provide instructions on how to test it: 1. If you add a feature or fix a bug, add a test to cover your changes. 2. If you fix a flaky test, repeat it for many times to prove it works.) --------- Co-authored-by: wangqi <wangqi@xinxuan.net>

… catalog (apache#3384)  ### What changes were proposed in this pull request? Add a default optional properties for fileset catalog <img width="597" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/1fe02e25-d80e-4182-bb47-6079b956fe39"> Remove fix width of version component <img width="289" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/cbc690cd-d721-4916-9d5d-1dea077499d5"> <img width="335" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/19ab6609-32cf-44e1-9bc0-e2eb12241ab0"> ### Why are the changes needed? Although the location property is optional, it is better to be there for ease of use The width of version component is should be flexible Fix: apache#3257 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? manual

…ache#3385)  ### What changes were proposed in this pull request? disable the metalake select with only one <img width="505" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/f91df10f-3856-4947-a8cb-6e6cdbe004f3"> ### Why are the changes needed? With only one metalake UI presents user with a choice Fix: apache#3285 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? manual

### What changes were proposed in this pull request? Add PoAn Yang (FrankYang0529) to Contributors list. Signed-off-by: PoAn Yang <payang@apache.org>

…connector (apache#2635)  ### What changes were proposed in this pull request? - support GravitinoCatalogStore to register the catalog. In the MR, we will support to create the hive catalog. ### Why are the changes needed? - Fix apache#3362 ### Does this PR introduce _any_ user-facing change? - support flink in gravitino ### How was this patch tested? - add UTs

…ables (apache#3703)  ### What changes were proposed in this pull request? Add a Hive catalog property "list-all-tables". Using this property to control whether the Iceberg table is displayed in the Hive table list. ### Why are the changes needed? The bug is a schema has the Iceberg tables in the Hive catalog Fix: apache#3403 ### Does this PR introduce _any_ user-facing change? N/A. ### How was this patch tested? 1.create a hive catalog with "list-all-tables " property. 2.crate a database and a iceberg table in the catalog by hive beeline 3.check whether the table is displayed in the catalog . --------- Co-authored-by: ericqin <ericqin@tencent.com>

### What changes were proposed in this pull request? orignal -> original

### What changes were proposed in this pull request? orignal -> original

### What changes were proposed in this pull request? Fixed an incorrect Java example. ### Why are the changes needed? Fix: #4077 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? No testing required Co-authored-by: rqyin <rqyin@easipass.com>

### What changes were proposed in this pull request? Fix irregular variable names: tablename -> tableName ### Why are the changes needed? Variable name `tablename` is irregular ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Modify variable name naming convention, no testing required.

…y environment variant (#4229)  ### What changes were proposed in this pull request? Read the environment variable (if exists) when -PskipDockerTests is not specified. ### Why are the changes needed? Fix: #4181 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? ``` export SKIP_DOCKER_TESTS=false ./gradlew build # check docket tests are running ```

… mutable (#4379)  ### What changes were proposed in this pull request? switch catalog properties value input disable to enable hive catalog before: <img width="623" alt="Pasted Graphic" src="https://github.com/user-attachments/assets/77472113-ddd8-4051-825a-21bbaaf07bdd"> hive catalog after: <img width="708" alt="Pasted Graphic 1" src="https://github.com/user-attachments/assets/d08634da-0044-452d-9ded-ff186aae3507"> Iceberg catalog before: <img width="623" alt="Pasted Graphic 2" src="https://github.com/user-attachments/assets/3ecca557-6b2d-42ab-8d0f-115044fe97b3"> iceberg catalog after: <img width="588" alt="Pasted Graphic 3" src="https://github.com/user-attachments/assets/5585d55c-4bec-42c3-b496-eaf75dc55e9f"> pg catalog before: <img width="580" alt="Pasted Graphic 6" src="https://github.com/user-attachments/assets/334abb54-ad33-42b5-9d96-6f3d7fdded19"> pg catalog after: <img width="594" alt="Pasted Graphic 7" src="https://github.com/user-attachments/assets/4aca0e85-8ba2-4bbe-a19b-58c52685ae42"> kafka catalog before: <img width="605" alt="Pasted Graphic 4" src="https://github.com/user-attachments/assets/37f1938d-90df-45ff-81bf-7081238921db"> kafka catalog after: <img width="592" alt="Pasted Graphic 5" src="https://github.com/user-attachments/assets/4dd7e321-dd6a-4e86-8e5d-dae6d601fbf7"> fileset catalog before: <img width="607" alt="Pasted Graphic 8" src="https://github.com/user-attachments/assets/1b430711-f494-4b87-89a2-ea9c774c381c"> fileset catalog after: <img width="618" alt="Pasted Graphic 9" src="https://github.com/user-attachments/assets/04ce17cb-5eac-4f25-b1e0-e309db2a1103"> ### Why are the changes needed? make the change by the [PR](#4262) merged Fix: #4351 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? manual Co-authored-by: Qian Xia <lauraxiaqian@gmail.com>

### What changes were proposed in this pull request? Add `**/*.out` to the list of exclusions will skip the licence check on the *.out file. ### Why are the changes needed? Fix: #4439 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? No Co-authored-by: Saker <46983675+IamSaker@users.noreply.github.com>

#4667)  ### What changes were proposed in this pull request? The documents should be updated. ### Why are the changes needed? To correct reference from MySQL to PostgreSQL in JDBC catalog documentation. Fix: #4666 ### Does this PR introduce _any_ user-facing change? No; Just the markdown documents. ### How was this patch tested? Manual review.

### What changes were proposed in this pull request? `TestHiveTableOperations` can be tested before `TestHiveTable` initialized. Make `TestHiveTableOperations::steup()` run alone. ### Why are the changes needed? When we try to just run `gradle :catalogs:catalog-hive:test --tests 'org.apache.gravitino.catalog.hive.TestHiveTableOperations'`, `TestHiveTableOperations` needs static variables which must be initialized ahead, and it can't be run alone. Fix: #4851 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Refactor of unit test setup, local unit tests have passed.

### What changes were proposed in this pull request? In `DorisTableOperations`, handled a possible Null Pointer Exception scenario ### Why are the changes needed? Fix: (#4605) ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Local Test Run - Passed --------- Co-authored-by: Avinash <AMoorthy@csod.com>

[Subtask] Add KV backend storage interface for EntityStore

0948ade

yuqi1129 requested a review from jerryshao July 25, 2023 12:29

[Subtask] Add KV backend storage interface for EntityStore, fix check…

f67fdab

…style problem

yuqi1129 mentioned this pull request Jul 25, 2023

[#121] feat(storage): Implement RocksDB storage backend for EntityStore #125

Merged

jerryshao reviewed Jul 26, 2023

View reviewed changes

[Subtask] Fix discussion

17ab6fa

jerryshao assigned yuqi1129 Jul 26, 2023

jerryshao approved these changes Jul 26, 2023

View reviewed changes

jerryshao merged commit 8b7c919 into apache:main Jul 26, 2023
1 check passed

liujinhui1994 mentioned this pull request Mar 21, 2024

[#2625] fix: cannot compile under docker bash #2626

Closed

yuqi1129 mentioned this pull request Jul 8, 2024

[TEST] Test authentication #4103

Closed

yuqi1129 mentioned this pull request Aug 14, 2024

[#4445] feat(docker): Publish Docker image to Apache official DockerH… #4522

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#122] feat(core): Add KV backend storage interface for EntityStore #123

[#122] feat(core): Add KV backend storage interface for EntityStore #123

yuqi1129 commented Jul 25, 2023

jerryshao Jul 26, 2023

yuqi1129 Jul 26, 2023

jerryshao commented Jul 26, 2023

[#122] feat(core): Add KV backend storage interface for EntityStore #123

[#122] feat(core): Add KV backend storage interface for EntityStore #123

Conversation

yuqi1129 commented Jul 25, 2023

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

jerryshao Jul 26, 2023

Choose a reason for hiding this comment

yuqi1129 Jul 26, 2023

Choose a reason for hiding this comment

jerryshao commented Jul 26, 2023