Skip to content

Commit

Permalink
[apache#1772] improvement(conf): Change the default storage directroy…
Browse files Browse the repository at this point in the history
… of `rocksdbPath` in gravitino.conf.template and support relative path (apache#1773)

### What changes were proposed in this pull request?

Change the value of `gravitino.entity.store.kv.rocksdbPath` from
`/tmp/gravitino` to `${GRAVITINO_HOME}/data/rocksdb`

### Why are the changes needed?

`/tmp/rocksdb` is misleading as it's not the default value for it.

Fix: apache#1772 

### Does this PR introduce _any_ user-facing change?

N/A.

### How was this patch tested?

N/A.
  • Loading branch information
yuqi1129 authored Feb 21, 2024
1 parent 3b7e01a commit 6d5bb64
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 12 deletions.
6 changes: 4 additions & 2 deletions conf/gravitino.conf.template
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ gravitino.server.webserver.responseHeaderSize = 131072
gravitino.entity.store = kv
# The RocksDB entity store
gravitino.entity.store.kv = RocksDBKvBackend
# The RocksDB backend path for entity store
# gravitino.entity.store.kv.rocksdbPath = /tmp/gravitino
# The storage path for RocksDB storage implementation, it supports both absolute and relative path,
# If the value is a relative path, the final path is "${GRAVITINO_HOME}/${PATH_YOU_HAVA_SET}", default value
# is "${GRAVITINO_HOME}/data/rocksdb", please uncomment and change it in your production environment.
# gravitino.entity.store.kv.rocksdbPath = data/rocksdb

# THE CONFIGURATION FOR Gravitino CATALOG
# The interval in milliseconds to evict the catalog cache
Expand Down
6 changes: 5 additions & 1 deletion core/src/main/java/com/datastrato/gravitino/Configs.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@ public interface Configs {

ConfigEntry<String> ENTRY_KV_ROCKSDB_BACKEND_PATH =
new ConfigBuilder(ENTITY_KV_ROCKSDB_BACKEND_PATH_KEY)
.doc("Directory path of `RocksDBKvBackend`")
.doc(
"The storage path for RocksDB storage implementation. It supports both absolute and"
+ " relative path, if the value is a relative path, the final path is "
+ "`${GRAVITINO_HOME}/${PATH_YOU_HAVA_SET}`, default value is "
+ "`${GRAVITINO_HOME}/data/rocksdb`")
.version(ConfigConstants.VERSION_0_1_0)
.stringConf()
.createWithDefault(DEFAULT_KV_ROCKSDB_BACKEND_PATH);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
import com.google.common.collect.Lists;
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.tuple.Pair;
import org.rocksdb.Options;
import org.rocksdb.RocksDB;
Expand All @@ -40,7 +43,7 @@ public class RocksDBKvBackend implements KvBackend {
private RocksDB initRocksDB(Config config) throws RocksDBException {
RocksDB.loadLibrary();

String dbPath = config.get(Configs.ENTRY_KV_ROCKSDB_BACKEND_PATH);
String dbPath = getStoragePath(config);
File dbDir = new File(dbPath, "instance");
try (final Options options = new Options()) {
options.setCreateIfMissing(true);
Expand All @@ -62,6 +65,23 @@ private RocksDB initRocksDB(Config config) throws RocksDBException {
}
}

@VisibleForTesting
String getStoragePath(Config config) {
String dbPath = config.get(Configs.ENTRY_KV_ROCKSDB_BACKEND_PATH);
if (StringUtils.isBlank(dbPath)) {
return Configs.DEFAULT_KV_ROCKSDB_BACKEND_PATH;
}

Path path = Paths.get(dbPath);
// Relative Path
if (!path.isAbsolute()) {
path = Paths.get(System.getenv("GRAVITINO_HOME"), dbPath);
return path.toString();
}

return dbPath;
}

@Override
public void initialize(Config config) throws IOException {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,21 @@ private KvBackend getKvBackEnd() throws IOException {
return kvBackend;
}

@Test
void testStoragePath() {
Config config = Mockito.mock(Config.class);
Mockito.when(config.get(ENTRY_KV_ROCKSDB_BACKEND_PATH)).thenReturn("/a/b");
RocksDBKvBackend kvBackend = new RocksDBKvBackend();
String path = kvBackend.getStoragePath(config);
Assertions.assertEquals("/a/b", path);

Mockito.when(config.get(ENTRY_KV_ROCKSDB_BACKEND_PATH)).thenReturn("");
kvBackend = new RocksDBKvBackend();
path = kvBackend.getStoragePath(config);
// We haven't set the GRAVITINO_HOME
Assertions.assertEquals("null/data/rocksdb", path);
}

@Test
void testPutAndGet() throws IOException, RocksDBException {
KvBackend kvBackend = getKvBackEnd();
Expand Down
16 changes: 8 additions & 8 deletions docs/gravitino-server-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ You can also specify filter parameters by setting configuration entries of the f

### Storage configuration

| Configuration item | Description | Default value | Required | Since version |
|---------------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------|----------------------------------|----------|---------------|
| `gravitino.entity.store` | Which storage implementation to use. Key-value pair storage is currently supported, and the default value is `kv`. | `kv` | No | 0.1.0 |
| `gravitino.entity.store.kv` | Detailed implementation of KV storage. `RocksDB` storage is currently supported, and the implementation is `RocksDBKvBackend`. | `RocksDBKvBackend` | No | 0.1.0 |
| `gravitino.entity.store.kv.rocksdbPath` | Directory path of `RocksDBKvBackend`. | `${GRAVITINO_HOME}/data/rocksdb` | No | 0.1.0 |
| `graivitino.entity.serde` | The serialization/deserialization class used to support entity storage. `proto' is currently supported. | `proto` | No | 0.1.0 |
| `gravitino.entity.store.maxTransactionSkewTimeMs` | The maximum skew time of transactions in milliseconds. | `2000` | No | 0.3.0 |
| `gravitino.entity.store.kv.deleteAfterTimeMs` | The maximum time in milliseconds that deleted and old-version data is kept. Set to at least 10 minutes and no longer than 30 days. | `604800000`(7 days) | No | 0.3.0 |
| Configuration item | Description | Default value | Required | Since version |
|---------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------------------------------|----------|---------------|
| `gravitino.entity.store` | Which storage implementation to use. Key-value pair storage is currently supported, and the default value is `kv`. | `kv` | No | 0.1.0 |
| `gravitino.entity.store.kv` | Detailed implementation of KV storage. `RocksDB` storage is currently supported, and the implementation is `RocksDBKvBackend`. | `RocksDBKvBackend` | No | 0.1.0 |
| `gravitino.entity.store.kv.rocksdbPath` | The storage path for RocksDB storage implementation. It supports both absolute and relative path, if the value is a relative path, the final path is `${GRAVITINO_HOME}/${PATH_YOU_HAVA_SET}`, default value is `${GRAVITINO_HOME}/data/rocksdb` | `${GRAVITINO_HOME}/data/rocksdb` | No | 0.1.0 |
| `graivitino.entity.serde` | The serialization/deserialization class used to support entity storage. `proto' is currently supported. | `proto` | No | 0.1.0 |
| `gravitino.entity.store.maxTransactionSkewTimeMs` | The maximum skew time of transactions in milliseconds. | `2000` | No | 0.3.0 |
| `gravitino.entity.store.kv.deleteAfterTimeMs` | The maximum time in milliseconds that deleted and old-version data is kept. Set to at least 10 minutes and no longer than 30 days. | `604800000`(7 days) | No | 0.3.0 |

:::caution
We strongly recommend that you change the default value of `gravitino.entity.store.kv.rocksdbPath`, as it's under the deployment directory and future version upgrades may remove it.
Expand Down

0 comments on commit 6d5bb64

Please sign in to comment.