dxichen commented on code in PR #1706:
URL: https://github.com/apache/samza/pull/1706#discussion_r1727615714
##########
samza-core/src/main/java/org/apache/samza/config/StorageConfig.java:
##########
@@ -263,6 +268,14 @@ public long getChangelogMinCompactionLagMs(String
storeName) {
return getLong(minCompactLagConfigName,
getDefaultChangelogMinCompactionLagMs());
}
+ /**
+ * Helper method to get the default RocksDB max manifest file size in bytes
for ALL stores, which is default to 1GB.
Review Comment:
s/ALL stores/ANY store
All makes it ambiguous whether it is the sum of all stores or individual size
##########
samza-core/src/main/java/org/apache/samza/config/StorageConfig.java:
##########
@@ -79,7 +81,10 @@ public class StorageConfig extends MapConfig {
public static final String STORE_RESTORE_FACTORIES = STORE_PREFIX + "%s." +
RESTORE_FACTORIES_SUFFIX;
public static final String JOB_RESTORE_FACTORIES = STORE_PREFIX +
RESTORE_FACTORIES_SUFFIX;
public static final List<String> DEFAULT_RESTORE_FACTORIES =
ImmutableList.of(KAFKA_STATE_BACKEND_FACTORY);
+ public static final long DEFAULT_ROCKSDB_MAX_MANIFEST_FILE_SIZE_IN_BYTES =
1024 * 1024 * 1024L;
+ static final String DEFAULT_ROCKSDB_MAX_MANIFEST_FILE_SIZE =
+ STORE_PREFIX + DEFAULT_STORE_NAME + ".rocksdb.max.manifest.file.size";
Review Comment:
for the `default` config, lets not follow the same format as the config with
named stores in case of `default` table name collision
##########
docs/learn/documentation/versioned/jobs/samza-configurations.md:
##########
@@ -321,7 +321,8 @@ These properties define Samza's storage mechanism for
efficient [stateful stream
|stores.**_store-name_**.<br>rocksdb.keep.log.file.num|2|The number of RocksDB
LOG files (including rotated LOG.old.* files) to keep.|
|stores.**_store-name_**.<br>rocksdb.metrics.list|(none)|A list of [RocksDB
properties](https://github.com/facebook/rocksdb/blob/master/include/rocksdb/db.h#L409)
to expose as metrics (gauges).|
|stores.**_store-name_**.<br>rocksdb.delete.obsolete.files.period.micros|21600000000|This
property specifies the period in microseconds to delete obsolete files
regardless of files removed during compaction. Allowed range is up to
9223372036854775807.|
-|stores.**_store-name_**.<br>rocksdb.max.manifest.file.size|18446744073709551615|This
property specifies the maximum size of the MANIFEST data file, after which it
is rotated. Default value is also the maximum, making it practically unlimited:
only one manifest file is used.|
+|stores.default.<br>rocksdb.max.manifest.file.size|1073741824| This property
specifies the default maximum size of the MANIFEST data file for **ALL**
stores, after which it is rotated. The default value is 1GB. The value will be
overriden by the per-store configuration with
`stores.store-name.rocksdb.max.manifest.file.size`.
|
Review Comment:
Please ad the unit of the size here
##########
docs/learn/documentation/versioned/jobs/samza-configurations.md:
##########
@@ -321,7 +321,8 @@ These properties define Samza's storage mechanism for
efficient [stateful stream
|stores.**_store-name_**.<br>rocksdb.keep.log.file.num|2|The number of RocksDB
LOG files (including rotated LOG.old.* files) to keep.|
|stores.**_store-name_**.<br>rocksdb.metrics.list|(none)|A list of [RocksDB
properties](https://github.com/facebook/rocksdb/blob/master/include/rocksdb/db.h#L409)
to expose as metrics (gauges).|
|stores.**_store-name_**.<br>rocksdb.delete.obsolete.files.period.micros|21600000000|This
property specifies the period in microseconds to delete obsolete files
regardless of files removed during compaction. Allowed range is up to
9223372036854775807.|
-|stores.**_store-name_**.<br>rocksdb.max.manifest.file.size|18446744073709551615|This
property specifies the maximum size of the MANIFEST data file, after which it
is rotated. Default value is also the maximum, making it practically unlimited:
only one manifest file is used.|
+|stores.default.<br>rocksdb.max.manifest.file.size|1073741824| This property
specifies the default maximum size of the MANIFEST data file for **ALL**
stores, after which it is rotated. The default value is 1GB. The value will be
overriden by the per-store configuration with
`stores.store-name.rocksdb.max.manifest.file.size`.
|
Review Comment:
Please add the unit of the size here
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
