Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[server] Validate block-cache config against system memory #1508

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[server] Validate block-cache config against system memory #1508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2861744
[server] Validate block-cache config against system memory
Feb 6, 2025
f3ecda2
[server] Validate block-cache config against system memory
Feb 6, 2025
f7b9681
[server] Validate block-cache config against system memory
Feb 6, 2025
bc574b7
addressed comments
Feb 7, 2025
70246a3
updated tests
Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions ...-client/src/main/java/com/linkedin/davinci/store/rocksdb/RocksDBStorageEngineFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.linkedin.davinci.store.rocksdb;

import static com.linkedin.venice.utils.ByteUtils.generateHumanReadableByteCountString;
import static com.linkedin.venice.utils.Utils.getOSMemorySize;
import static org.rocksdb.RateLimiter.DEFAULT_FAIRNESS;
import static org.rocksdb.RateLimiter.DEFAULT_MODE;
import static org.rocksdb.RateLimiter.DEFAULT_REFILL_PERIOD_MICROS;
Expand Down Expand Up @@ -139,6 +141,18 @@ public RocksDBStorageEngineFactory(
this.env.setBackgroundThreads(rocksDBServerConfig.getRocksDBEnvFlushPoolSize(), Priority.HIGH);
this.env.setBackgroundThreads(rocksDBServerConfig.getRocksDBEnvCompactionPoolSize(), Priority.LOW);

long cacheBytesNeeded =
rocksDBServerConfig.getRocksDBBlockCacheSizeInBytes() + (rocksDBServerConfig.isUseSeparateRMDCacheEnabled()
? rocksDBServerConfig.getRocksDBRMDBlockCacheSizeInBytes()
: 0);

long systemMemorySize = getOSMemorySize();
if (systemMemorySize > 0 && (systemMemorySize * 0.8 < cacheBytesNeeded)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @m-nagarajan mentioned, it is better to make the ratio configurable.

throw new RuntimeException(
"Cannot setup rocksdb instance with block-cache size "
+ generateHumanReadableByteCountString(cacheBytesNeeded) + ". System memory : "
+ generateHumanReadableByteCountString(systemMemorySize));
}
// Shared cache across all the RocksDB databases
if (RocksDBBlockCacheImplementations.CLOCK.equals(rocksDBServerConfig.getRocksDBBlockCacheImplementation())) {
if (rocksDBServerConfig.isUseSeparateRMDCacheEnabled()) {
Expand Down
4 changes: 4 additions & 0 deletions ...a/com/linkedin/davinci/store/rocksdb/ReplicationMeadataRocksDBStoragePartitionCFTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.linkedin.davinci.store.rocksdb;

Check failure on line 1 in clients/da-vinci-client/src/test/java/com/linkedin/davinci/store/rocksdb/ReplicationMeadataRocksDBStoragePartitionCFTest.java

View workflow job for this annotation

GitHub Actions / Server / UT & CodeCov (8)

ReplicationMeadataRocksDBStoragePartitionCFTest.testMetadataColumnFamily 

java.lang.RuntimeException: Cannot setup rocksdb instance with block-cache size 16.0 GiB. System memory : 15.6 GiB

Check failure on line 1 in clients/da-vinci-client/src/test/java/com/linkedin/davinci/store/rocksdb/ReplicationMeadataRocksDBStoragePartitionCFTest.java

View workflow job for this annotation

GitHub Actions / Server / UT & CodeCov (8)

ReplicationMeadataRocksDBStoragePartitionCFTest.testReplicationMetadataIngestion[0](true, false, false, true) 

java.lang.RuntimeException: Cannot setup rocksdb instance with block-cache size 16.0 GiB. System memory : 15.6 GiB

Check failure on line 1 in clients/da-vinci-client/src/test/java/com/linkedin/davinci/store/rocksdb/ReplicationMeadataRocksDBStoragePartitionCFTest.java

View workflow job for this annotation

GitHub Actions / Server / UT & CodeCov (8)

ReplicationMeadataRocksDBStoragePartitionCFTest.testReplicationMetadataIngestion[1](true, false, false, false) 

java.lang.RuntimeException: Cannot setup rocksdb instance with block-cache size 16.0 GiB. System memory : 15.6 GiB

Check failure on line 1 in clients/da-vinci-client/src/test/java/com/linkedin/davinci/store/rocksdb/ReplicationMeadataRocksDBStoragePartitionCFTest.java

View workflow job for this annotation

GitHub Actions / Server / UT & CodeCov (8)

ReplicationMeadataRocksDBStoragePartitionCFTest.testReplicationMetadataIngestion[2](true, true, true, false) 

java.lang.RuntimeException: Cannot setup rocksdb instance with block-cache size 16.0 GiB. System memory : 15.6 GiB

Check failure on line 1 in clients/da-vinci-client/src/test/java/com/linkedin/davinci/store/rocksdb/ReplicationMeadataRocksDBStoragePartitionCFTest.java

View workflow job for this annotation

GitHub Actions / Server / UT & CodeCov (8)

ReplicationMeadataRocksDBStoragePartitionCFTest.testReplicationMetadataIngestion[3](true, true, false, false) 

java.lang.RuntimeException: Cannot setup rocksdb instance with block-cache size 16.0 GiB. System memory : 15.6 GiB

Check failure on line 1 in clients/da-vinci-client/src/test/java/com/linkedin/davinci/store/rocksdb/ReplicationMeadataRocksDBStoragePartitionCFTest.java

View workflow job for this annotation

GitHub Actions / Server / UT & CodeCov (17)

ReplicationMeadataRocksDBStoragePartitionCFTest.testMetadataColumnFamily 

java.lang.RuntimeException: Cannot setup rocksdb instance with block-cache size 16.0 GiB. System memory : 15.6 GiB

Check failure on line 1 in clients/da-vinci-client/src/test/java/com/linkedin/davinci/store/rocksdb/ReplicationMeadataRocksDBStoragePartitionCFTest.java

View workflow job for this annotation

GitHub Actions / Server / UT & CodeCov (17)

ReplicationMeadataRocksDBStoragePartitionCFTest.testReplicationMetadataIngestion[0](true, false, false, true) 

java.lang.RuntimeException: Cannot setup rocksdb instance with block-cache size 16.0 GiB. System memory : 15.6 GiB

Check failure on line 1 in clients/da-vinci-client/src/test/java/com/linkedin/davinci/store/rocksdb/ReplicationMeadataRocksDBStoragePartitionCFTest.java

View workflow job for this annotation

GitHub Actions / Server / UT & CodeCov (17)

ReplicationMeadataRocksDBStoragePartitionCFTest.testReplicationMetadataIngestion[1](true, false, false, false) 

java.lang.RuntimeException: Cannot setup rocksdb instance with block-cache size 16.0 GiB. System memory : 15.6 GiB

Check failure on line 1 in clients/da-vinci-client/src/test/java/com/linkedin/davinci/store/rocksdb/ReplicationMeadataRocksDBStoragePartitionCFTest.java

View workflow job for this annotation

GitHub Actions / Server / UT & CodeCov (17)

ReplicationMeadataRocksDBStoragePartitionCFTest.testReplicationMetadataIngestion[2](true, true, true, false) 

java.lang.RuntimeException: Cannot setup rocksdb instance with block-cache size 16.0 GiB. System memory : 15.6 GiB

import static com.linkedin.davinci.store.rocksdb.RocksDBServerConfig.ROCKSDB_BLOCK_CACHE_SIZE_IN_BYTES;
import static com.linkedin.davinci.store.rocksdb.RocksDBServerConfig.ROCKSDB_RMD_BLOCK_CACHE_SIZE_IN_BYTES;
import static com.linkedin.davinci.store.rocksdb.RocksDBServerConfig.ROCKSDB_SEPARATE_RMD_CACHE_ENABLED;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
Expand Down Expand Up @@ -49,6 +51,8 @@

Properties properties = new Properties();
properties.put(ROCKSDB_SEPARATE_RMD_CACHE_ENABLED, "true");
properties.put(ROCKSDB_BLOCK_CACHE_SIZE_IN_BYTES, 1024 * 1024 * 1024L);
properties.put(ROCKSDB_RMD_BLOCK_CACHE_SIZE_IN_BYTES, 1024 * 1024L);
VeniceProperties serverProps = AbstractStorageEngineTest.getServerProperties(PersistenceType.ROCKS_DB, properties);
storageService = new StorageService(
AbstractStorageEngineTest.getVeniceConfigLoader(serverProps),
Expand Down
13 changes: 13 additions & 0 deletions internal/venice-common/src/main/java/com/linkedin/venice/utils/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.lang.management.ManagementFactory;
import java.lang.management.OperatingSystemMXBean;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.nio.file.Files;
Expand Down Expand Up @@ -1141,4 +1142,16 @@ public static long parseDateTimeToEpoch(String dateTime, String dateTimeFormat,
dateFormat.setTimeZone(TimeZone.getTimeZone(timeZone));
return dateFormat.parse(dateTime).getTime();
}

public static long getOSMemorySize() {
OperatingSystemMXBean osBean = ManagementFactory.getOperatingSystemMXBean();

if (osBean instanceof com.sun.management.OperatingSystemMXBean) {
com.sun.management.OperatingSystemMXBean extendedOsBean = (com.sun.management.OperatingSystemMXBean) osBean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tested this function in Mac or Linux with OpenJDK?
If it doesn't work in these envs, this feature will be useless...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this works in mac, and linux container/helix code uses same mechanism to fetch system memory.

return extendedOsBean.getTotalPhysicalMemorySize();
} else {
System.out.println("OS Bean not available.");
}
return -1;
}
}
2 changes: 2 additions & 0 deletions ...mmon/src/integrationTest/java/com/linkedin/venice/endToEnd/DaVinciClientDiskFullTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.linkedin.venice.endToEnd;

import static com.linkedin.davinci.store.rocksdb.RocksDBServerConfig.ROCKSDB_BLOCK_CACHE_SIZE_IN_BYTES;
import static com.linkedin.venice.ConfigKeys.CLIENT_SYSTEM_STORE_REPOSITORY_REFRESH_INTERVAL_SECONDS;
import static com.linkedin.venice.ConfigKeys.CLIENT_USE_SYSTEM_STORE_REPOSITORY;
import static com.linkedin.venice.ConfigKeys.CLUSTER_DISCOVERY_D2_SERVICE;
Expand Down Expand Up @@ -142,6 +143,7 @@ private VeniceProperties getDaVinciBackendConfig(boolean useDaVinciSpecificExecu
.put(PERSISTENCE_TYPE, ROCKS_DB)
.put(PUSH_STATUS_STORE_ENABLED, true)
.put(D2_ZK_HOSTS_ADDRESS, venice.getZk().getAddress())
.put(ROCKSDB_BLOCK_CACHE_SIZE_IN_BYTES, 2 * 1024 * 1024L)
.put(CLUSTER_DISCOVERY_D2_SERVICE, VeniceRouterWrapper.CLUSTER_DISCOVERY_D2_SERVICE_NAME)
.put(USE_DA_VINCI_SPECIFIC_EXECUTION_STATUS_FOR_ERROR, useDaVinciSpecificExecutionStatusForError)
.put(SERVER_DISK_FULL_THRESHOLD, getDiskFullThreshold(largePushRecordCount, largePushRecordMinSize));
Expand Down
2 changes: 2 additions & 0 deletions ...on/src/integrationTest/java/com/linkedin/venice/integration/utils/DaVinciTestContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.linkedin.venice.integration.utils;

import static com.linkedin.davinci.store.rocksdb.RocksDBServerConfig.ROCKSDB_BLOCK_CACHE_SIZE_IN_BYTES;
import static com.linkedin.venice.ConfigKeys.CLIENT_SYSTEM_STORE_REPOSITORY_REFRESH_INTERVAL_SECONDS;
import static com.linkedin.venice.ConfigKeys.CLIENT_USE_SYSTEM_STORE_REPOSITORY;
import static com.linkedin.venice.ConfigKeys.CLUSTER_DISCOVERY_D2_SERVICE;
Expand Down Expand Up @@ -132,6 +133,7 @@ public static PropertyBuilder getDaVinciPropertyBuilder(String zkAddress) {
.put(CLIENT_USE_SYSTEM_STORE_REPOSITORY, true)
.put(CLIENT_SYSTEM_STORE_REPOSITORY_REFRESH_INTERVAL_SECONDS, 1)
.put(D2_ZK_HOSTS_ADDRESS, zkAddress)
.put(ROCKSDB_BLOCK_CACHE_SIZE_IN_BYTES, 4 * 1024 * 1024 * 1024L)
.put(CLUSTER_DISCOVERY_D2_SERVICE, VeniceRouterWrapper.CLUSTER_DISCOVERY_D2_SERVICE_NAME);
}
}
4 changes: 4 additions & 0 deletions ...n/src/integrationTest/java/com/linkedin/venice/integration/utils/VeniceServerWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package com.linkedin.venice.integration.utils;

import static com.linkedin.davinci.store.rocksdb.RocksDBServerConfig.ROCKSDB_BLOCK_CACHE_SIZE_IN_BYTES;
import static com.linkedin.davinci.store.rocksdb.RocksDBServerConfig.ROCKSDB_OPTIONS_USE_DIRECT_READS;
import static com.linkedin.davinci.store.rocksdb.RocksDBServerConfig.ROCKSDB_PLAIN_TABLE_FORMAT_ENABLED;
import static com.linkedin.davinci.store.rocksdb.RocksDBServerConfig.ROCKSDB_RMD_BLOCK_CACHE_SIZE_IN_BYTES;
import static com.linkedin.venice.ConfigKeys.ADMIN_PORT;
import static com.linkedin.venice.ConfigKeys.CLUSTER_DISCOVERY_D2_SERVICE;
import static com.linkedin.venice.ConfigKeys.DATA_BASE_PATH;
Expand Down Expand Up @@ -264,6 +266,8 @@ static StatefulServiceProvider<VeniceServerWrapper> generateService(
.put(SERVER_INGESTION_HEARTBEAT_INTERVAL_MS, 5000)
.put(SERVER_LEADER_COMPLETE_STATE_CHECK_IN_FOLLOWER_VALID_INTERVAL_MS, 5000)
.put(SERVER_RESUBSCRIPTION_TRIGGERED_BY_VERSION_INGESTION_CONTEXT_CHANGE_ENABLED, true)
.put(ROCKSDB_BLOCK_CACHE_SIZE_IN_BYTES, 8 * 1024 * 1024 * 1024L)
.put(ROCKSDB_RMD_BLOCK_CACHE_SIZE_IN_BYTES, 1024 * 1024 * 1024L)
.put(SERVER_DELETE_UNASSIGNED_PARTITIONS_ON_STARTUP, serverDeleteUnassignedPartitionsOnStartup);
if (sslToKafka) {
serverPropsBuilder.put(KAFKA_SECURITY_PROTOCOL, PubSubSecurityProtocol.SSL.name());
Expand Down
2 changes: 2 additions & 0 deletions internal/venice-test-common/src/main/java/com/linkedin/venice/utils/TestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.linkedin.venice.utils;

import static com.linkedin.davinci.store.rocksdb.RocksDBServerConfig.ROCKSDB_BLOCK_CACHE_SIZE_IN_BYTES;
import static com.linkedin.venice.ConfigKeys.KAFKA_BOOTSTRAP_SERVERS;
import static com.linkedin.venice.ConfigKeys.PARTITIONER_CLASS;
import static com.linkedin.venice.ConfigKeys.SERVER_FORKED_PROCESS_JVM_ARGUMENT_LIST;
Expand Down Expand Up @@ -875,6 +876,7 @@ public static void shutdownExecutor(ExecutorService executor, long timeout, Time
public static Map<String, Object> getIngestionIsolationPropertyMap() {
Map<String, Object> propertyMap = new HashMap<>();
propertyMap.put(SERVER_INGESTION_MODE, IngestionMode.ISOLATED);
propertyMap.put(ROCKSDB_BLOCK_CACHE_SIZE_IN_BYTES, 1 * 1024 * 1024 * 1024L);
propertyMap.put(SERVER_FORKED_PROCESS_JVM_ARGUMENT_LIST, "-Xms256M;-Xmx1G");
return propertyMap;
}
Expand Down
Loading