gharris1727 commented on code in PR #16199: URL: https://github.com/apache/kafka/pull/16199#discussion_r1628030168
########## storage/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogManagerConfig.java: ########## @@ -348,280 +346,124 @@ public final class RemoteLogManagerConfig { REMOTE_LOG_MANAGER_FETCH_QUOTA_WINDOW_SIZE_SECONDS_DOC); } - private final boolean enableRemoteStorageSystem; - private final String remoteStorageManagerClassName; - private final String remoteStorageManagerClassPath; - private final String remoteLogMetadataManagerClassName; - private final String remoteLogMetadataManagerClassPath; - private final long remoteLogIndexFileCacheTotalSizeBytes; - private final int remoteLogManagerThreadPoolSize; - private final int remoteLogManagerCopierThreadPoolSize; - private final int remoteLogManagerExpirationThreadPoolSize; - private final long remoteLogManagerTaskIntervalMs; - private final long remoteLogManagerTaskRetryBackoffMs; - private final long remoteLogManagerTaskRetryBackoffMaxMs; - private final double remoteLogManagerTaskRetryJitter; - private final int remoteLogReaderThreads; - private final int remoteLogReaderMaxPendingTasks; - private final String remoteStorageManagerPrefix; - private final HashMap<String, Object> remoteStorageManagerProps; - private final String remoteLogMetadataManagerPrefix; - private final HashMap<String, Object> remoteLogMetadataManagerProps; - private final String remoteLogMetadataManagerListenerName; - private final int remoteLogMetadataCustomMetadataMaxBytes; - private final long remoteLogManagerCopyMaxBytesPerSecond; - private final int remoteLogManagerCopyNumQuotaSamples; - private final int remoteLogManagerCopyQuotaWindowSizeSeconds; - private final long remoteLogManagerFetchMaxBytesPerSecond; - private final int remoteLogManagerFetchNumQuotaSamples; - private final int remoteLogManagerFetchQuotaWindowSizeSeconds; - - public RemoteLogManagerConfig(AbstractConfig config) { - this(config.getBoolean(REMOTE_LOG_STORAGE_SYSTEM_ENABLE_PROP), - config.getString(REMOTE_STORAGE_MANAGER_CLASS_NAME_PROP), - config.getString(REMOTE_STORAGE_MANAGER_CLASS_PATH_PROP), - config.getString(REMOTE_LOG_METADATA_MANAGER_CLASS_NAME_PROP), - config.getString(REMOTE_LOG_METADATA_MANAGER_CLASS_PATH_PROP), - config.getString(REMOTE_LOG_METADATA_MANAGER_LISTENER_NAME_PROP), - config.getLong(REMOTE_LOG_INDEX_FILE_CACHE_TOTAL_SIZE_BYTES_PROP), - config.getInt(REMOTE_LOG_MANAGER_THREAD_POOL_SIZE_PROP), - config.getInt(REMOTE_LOG_MANAGER_COPIER_THREAD_POOL_SIZE_PROP), - config.getInt(REMOTE_LOG_MANAGER_EXPIRATION_THREAD_POOL_SIZE_PROP), - config.getLong(REMOTE_LOG_MANAGER_TASK_INTERVAL_MS_PROP), - config.getLong(REMOTE_LOG_MANAGER_TASK_RETRY_BACK_OFF_MS_PROP), - config.getLong(REMOTE_LOG_MANAGER_TASK_RETRY_BACK_OFF_MAX_MS_PROP), - config.getDouble(REMOTE_LOG_MANAGER_TASK_RETRY_JITTER_PROP), - config.getInt(REMOTE_LOG_READER_THREADS_PROP), - config.getInt(REMOTE_LOG_READER_MAX_PENDING_TASKS_PROP), - config.getInt(REMOTE_LOG_METADATA_CUSTOM_METADATA_MAX_BYTES_PROP), - config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP), - config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP) != null - ? config.originalsWithPrefix(config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP)) - : Collections.emptyMap(), - config.getString(REMOTE_LOG_METADATA_MANAGER_CONFIG_PREFIX_PROP), - config.getString(REMOTE_LOG_METADATA_MANAGER_CONFIG_PREFIX_PROP) != null - ? config.originalsWithPrefix(config.getString(REMOTE_LOG_METADATA_MANAGER_CONFIG_PREFIX_PROP)) - : Collections.emptyMap(), - config.getLong(REMOTE_LOG_MANAGER_COPY_MAX_BYTES_PER_SECOND_PROP), - config.getInt(REMOTE_LOG_MANAGER_COPY_QUOTA_WINDOW_NUM_PROP), - config.getInt(REMOTE_LOG_MANAGER_COPY_QUOTA_WINDOW_SIZE_SECONDS_PROP), - config.getLong(REMOTE_LOG_MANAGER_FETCH_MAX_BYTES_PER_SECOND_PROP), - config.getInt(REMOTE_LOG_MANAGER_FETCH_QUOTA_WINDOW_NUM_PROP), - config.getInt(REMOTE_LOG_MANAGER_FETCH_QUOTA_WINDOW_SIZE_SECONDS_PROP)); - } - - // Visible for testing - public RemoteLogManagerConfig(boolean enableRemoteStorageSystem, - String remoteStorageManagerClassName, - String remoteStorageManagerClassPath, - String remoteLogMetadataManagerClassName, - String remoteLogMetadataManagerClassPath, - String remoteLogMetadataManagerListenerName, - long remoteLogIndexFileCacheTotalSizeBytes, - int remoteLogManagerThreadPoolSize, - int remoteLogManagerCopierThreadPoolSize, - int remoteLogManagerExpirationThreadPoolSize, - long remoteLogManagerTaskIntervalMs, - long remoteLogManagerTaskRetryBackoffMs, - long remoteLogManagerTaskRetryBackoffMaxMs, - double remoteLogManagerTaskRetryJitter, - int remoteLogReaderThreads, - int remoteLogReaderMaxPendingTasks, - int remoteLogMetadataCustomMetadataMaxBytes, - String remoteStorageManagerPrefix, - Map<String, Object> remoteStorageManagerProps, /* properties having keys stripped out with remoteStorageManagerPrefix */ - String remoteLogMetadataManagerPrefix, - Map<String, Object> remoteLogMetadataManagerProps, /* properties having keys stripped out with remoteLogMetadataManagerPrefix */ - long remoteLogManagerCopyMaxBytesPerSecond, - int remoteLogManagerCopyNumQuotaSamples, - int remoteLogManagerCopyQuotaWindowSizeSeconds, - long remoteLogManagerFetchMaxBytesPerSecond, - int remoteLogManagerFetchNumQuotaSamples, - int remoteLogManagerFetchQuotaWindowSizeSeconds - ) { - this.enableRemoteStorageSystem = enableRemoteStorageSystem; - this.remoteStorageManagerClassName = remoteStorageManagerClassName; - this.remoteStorageManagerClassPath = remoteStorageManagerClassPath; - this.remoteLogMetadataManagerClassName = remoteLogMetadataManagerClassName; - this.remoteLogMetadataManagerClassPath = remoteLogMetadataManagerClassPath; - this.remoteLogIndexFileCacheTotalSizeBytes = remoteLogIndexFileCacheTotalSizeBytes; - this.remoteLogManagerThreadPoolSize = remoteLogManagerThreadPoolSize; - this.remoteLogManagerCopierThreadPoolSize = remoteLogManagerCopierThreadPoolSize; - this.remoteLogManagerExpirationThreadPoolSize = remoteLogManagerExpirationThreadPoolSize; - this.remoteLogManagerTaskIntervalMs = remoteLogManagerTaskIntervalMs; - this.remoteLogManagerTaskRetryBackoffMs = remoteLogManagerTaskRetryBackoffMs; - this.remoteLogManagerTaskRetryBackoffMaxMs = remoteLogManagerTaskRetryBackoffMaxMs; - this.remoteLogManagerTaskRetryJitter = remoteLogManagerTaskRetryJitter; - this.remoteLogReaderThreads = remoteLogReaderThreads; - this.remoteLogReaderMaxPendingTasks = remoteLogReaderMaxPendingTasks; - this.remoteStorageManagerPrefix = remoteStorageManagerPrefix; - this.remoteStorageManagerProps = new HashMap<>(remoteStorageManagerProps); - this.remoteLogMetadataManagerPrefix = remoteLogMetadataManagerPrefix; - this.remoteLogMetadataManagerProps = new HashMap<>(remoteLogMetadataManagerProps); - this.remoteLogMetadataManagerListenerName = remoteLogMetadataManagerListenerName; - this.remoteLogMetadataCustomMetadataMaxBytes = remoteLogMetadataCustomMetadataMaxBytes; - this.remoteLogManagerCopyMaxBytesPerSecond = remoteLogManagerCopyMaxBytesPerSecond; - this.remoteLogManagerCopyNumQuotaSamples = remoteLogManagerCopyNumQuotaSamples; - this.remoteLogManagerCopyQuotaWindowSizeSeconds = remoteLogManagerCopyQuotaWindowSizeSeconds; - this.remoteLogManagerFetchMaxBytesPerSecond = remoteLogManagerFetchMaxBytesPerSecond; - this.remoteLogManagerFetchNumQuotaSamples = remoteLogManagerFetchNumQuotaSamples; - this.remoteLogManagerFetchQuotaWindowSizeSeconds = remoteLogManagerFetchQuotaWindowSizeSeconds; - } - public boolean enableRemoteStorageSystem() { - return enableRemoteStorageSystem; + return getBoolean(REMOTE_LOG_STORAGE_SYSTEM_ENABLE_PROP); } public String remoteStorageManagerClassName() { - return remoteStorageManagerClassName; + return getString(REMOTE_STORAGE_MANAGER_CLASS_NAME_PROP); } public String remoteStorageManagerClassPath() { - return remoteStorageManagerClassPath; + return getString(REMOTE_STORAGE_MANAGER_CLASS_PATH_PROP); } public String remoteLogMetadataManagerClassName() { - return remoteLogMetadataManagerClassName; + return getString(REMOTE_LOG_METADATA_MANAGER_CLASS_NAME_PROP); } public String remoteLogMetadataManagerClassPath() { - return remoteLogMetadataManagerClassPath; + return getString(REMOTE_LOG_METADATA_MANAGER_CLASS_PATH_PROP); } public long remoteLogIndexFileCacheTotalSizeBytes() { - return remoteLogIndexFileCacheTotalSizeBytes; + return getLong(REMOTE_LOG_INDEX_FILE_CACHE_TOTAL_SIZE_BYTES_PROP); } public int remoteLogManagerThreadPoolSize() { - return remoteLogManagerThreadPoolSize; + return getInt(REMOTE_LOG_MANAGER_THREAD_POOL_SIZE_PROP); } public int remoteLogManagerCopierThreadPoolSize() { - return remoteLogManagerCopierThreadPoolSize; + return getInt(REMOTE_LOG_MANAGER_COPIER_THREAD_POOL_SIZE_PROP); } public int remoteLogManagerExpirationThreadPoolSize() { - return remoteLogManagerExpirationThreadPoolSize; + return getInt(REMOTE_LOG_MANAGER_EXPIRATION_THREAD_POOL_SIZE_PROP); } public long remoteLogManagerTaskIntervalMs() { - return remoteLogManagerTaskIntervalMs; + return getLong(REMOTE_LOG_MANAGER_TASK_INTERVAL_MS_PROP); } public long remoteLogManagerTaskRetryBackoffMs() { - return remoteLogManagerTaskRetryBackoffMs; + return getLong(REMOTE_LOG_MANAGER_TASK_RETRY_BACK_OFF_MS_PROP); } public long remoteLogManagerTaskRetryBackoffMaxMs() { - return remoteLogManagerTaskRetryBackoffMaxMs; + return getLong(REMOTE_LOG_MANAGER_TASK_RETRY_BACK_OFF_MAX_MS_PROP); } public double remoteLogManagerTaskRetryJitter() { - return remoteLogManagerTaskRetryJitter; + return getDouble(REMOTE_LOG_MANAGER_TASK_RETRY_JITTER_PROP); } public int remoteLogReaderThreads() { - return remoteLogReaderThreads; + return getInt(REMOTE_LOG_READER_THREADS_PROP); } public int remoteLogReaderMaxPendingTasks() { - return remoteLogReaderMaxPendingTasks; + return getInt(REMOTE_LOG_READER_MAX_PENDING_TASKS_PROP); } public String remoteLogMetadataManagerListenerName() { - return remoteLogMetadataManagerListenerName; + return getString(REMOTE_LOG_METADATA_MANAGER_LISTENER_NAME_PROP); } public int remoteLogMetadataCustomMetadataMaxBytes() { - return remoteLogMetadataCustomMetadataMaxBytes; + return getInt(REMOTE_LOG_METADATA_CUSTOM_METADATA_MAX_BYTES_PROP); } public String remoteStorageManagerPrefix() { - return remoteStorageManagerPrefix; + return getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP); } public String remoteLogMetadataManagerPrefix() { - return remoteLogMetadataManagerPrefix; + return getString(REMOTE_LOG_METADATA_MANAGER_CONFIG_PREFIX_PROP); } public Map<String, Object> remoteStorageManagerProps() { - return Collections.unmodifiableMap(remoteStorageManagerProps); + return Collections.unmodifiableMap(getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP) != null + ? originalsWithPrefix(getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP)) + : Collections.emptyMap()); } public Map<String, Object> remoteLogMetadataManagerProps() { - return Collections.unmodifiableMap(remoteLogMetadataManagerProps); + return Collections.unmodifiableMap(getString(REMOTE_LOG_METADATA_MANAGER_CONFIG_PREFIX_PROP) != null + ? originalsWithPrefix(getString(REMOTE_LOG_METADATA_MANAGER_CONFIG_PREFIX_PROP)) + : Collections.emptyMap()); } public long remoteLogManagerCopyMaxBytesPerSecond() { - return remoteLogManagerCopyMaxBytesPerSecond; + return getLong(REMOTE_LOG_MANAGER_COPY_MAX_BYTES_PER_SECOND_PROP); } public int remoteLogManagerCopyNumQuotaSamples() { - return remoteLogManagerCopyNumQuotaSamples; + return getInt(REMOTE_LOG_MANAGER_COPY_QUOTA_WINDOW_NUM_PROP); } public int remoteLogManagerCopyQuotaWindowSizeSeconds() { - return remoteLogManagerCopyQuotaWindowSizeSeconds; + return getInt(REMOTE_LOG_MANAGER_COPY_QUOTA_WINDOW_SIZE_SECONDS_PROP); } public long remoteLogManagerFetchMaxBytesPerSecond() { - return remoteLogManagerFetchMaxBytesPerSecond; + return getLong(REMOTE_LOG_MANAGER_FETCH_MAX_BYTES_PER_SECOND_PROP); } public int remoteLogManagerFetchNumQuotaSamples() { - return remoteLogManagerFetchNumQuotaSamples; + return getInt(REMOTE_LOG_MANAGER_FETCH_QUOTA_WINDOW_NUM_PROP); } public int remoteLogManagerFetchQuotaWindowSizeSeconds() { - return remoteLogManagerFetchQuotaWindowSizeSeconds; + return getInt(REMOTE_LOG_MANAGER_FETCH_QUOTA_WINDOW_SIZE_SECONDS_PROP); } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (!(o instanceof RemoteLogManagerConfig)) return false; - RemoteLogManagerConfig that = (RemoteLogManagerConfig) o; - return enableRemoteStorageSystem == that.enableRemoteStorageSystem - && remoteLogIndexFileCacheTotalSizeBytes == that.remoteLogIndexFileCacheTotalSizeBytes - && remoteLogManagerThreadPoolSize == that.remoteLogManagerThreadPoolSize - && remoteLogManagerCopierThreadPoolSize == that.remoteLogManagerCopierThreadPoolSize - && remoteLogManagerExpirationThreadPoolSize == that.remoteLogManagerExpirationThreadPoolSize - && remoteLogManagerTaskIntervalMs == that.remoteLogManagerTaskIntervalMs - && remoteLogManagerTaskRetryBackoffMs == that.remoteLogManagerTaskRetryBackoffMs - && remoteLogManagerTaskRetryBackoffMaxMs == that.remoteLogManagerTaskRetryBackoffMaxMs - && remoteLogManagerTaskRetryJitter == that.remoteLogManagerTaskRetryJitter - && remoteLogReaderThreads == that.remoteLogReaderThreads - && remoteLogReaderMaxPendingTasks == that.remoteLogReaderMaxPendingTasks - && remoteLogMetadataCustomMetadataMaxBytes == that.remoteLogMetadataCustomMetadataMaxBytes - && Objects.equals(remoteStorageManagerClassName, that.remoteStorageManagerClassName) - && Objects.equals(remoteStorageManagerClassPath, that.remoteStorageManagerClassPath) - && Objects.equals(remoteLogMetadataManagerClassName, that.remoteLogMetadataManagerClassName) - && Objects.equals(remoteLogMetadataManagerClassPath, that.remoteLogMetadataManagerClassPath) - && Objects.equals(remoteLogMetadataManagerListenerName, that.remoteLogMetadataManagerListenerName) - && Objects.equals(remoteStorageManagerProps, that.remoteStorageManagerProps) - && Objects.equals(remoteLogMetadataManagerProps, that.remoteLogMetadataManagerProps) - && Objects.equals(remoteStorageManagerPrefix, that.remoteStorageManagerPrefix) - && Objects.equals(remoteLogMetadataManagerPrefix, that.remoteLogMetadataManagerPrefix) - && remoteLogManagerCopyMaxBytesPerSecond == that.remoteLogManagerCopyMaxBytesPerSecond - && remoteLogManagerCopyNumQuotaSamples == that.remoteLogManagerCopyNumQuotaSamples - && remoteLogManagerCopyQuotaWindowSizeSeconds == that.remoteLogManagerCopyQuotaWindowSizeSeconds - && remoteLogManagerFetchMaxBytesPerSecond == that.remoteLogManagerFetchMaxBytesPerSecond - && remoteLogManagerFetchNumQuotaSamples == that.remoteLogManagerFetchNumQuotaSamples - && remoteLogManagerFetchQuotaWindowSizeSeconds == that.remoteLogManagerFetchQuotaWindowSizeSeconds; + public RemoteLogManagerConfig(Map<?, ?> props) { + this(props, false); } - @Override - public int hashCode() { - return Objects.hash(enableRemoteStorageSystem, remoteStorageManagerClassName, remoteStorageManagerClassPath, - remoteLogMetadataManagerClassName, remoteLogMetadataManagerClassPath, remoteLogMetadataManagerListenerName, - remoteLogMetadataCustomMetadataMaxBytes, remoteLogIndexFileCacheTotalSizeBytes, remoteLogManagerThreadPoolSize, - remoteLogManagerCopierThreadPoolSize, remoteLogManagerExpirationThreadPoolSize, remoteLogManagerTaskIntervalMs, - remoteLogManagerTaskRetryBackoffMs, remoteLogManagerTaskRetryBackoffMaxMs, remoteLogManagerTaskRetryJitter, - remoteLogReaderThreads, remoteLogReaderMaxPendingTasks, remoteStorageManagerProps, remoteLogMetadataManagerProps, - remoteStorageManagerPrefix, remoteLogMetadataManagerPrefix, remoteLogManagerCopyMaxBytesPerSecond, - remoteLogManagerCopyNumQuotaSamples, remoteLogManagerCopyQuotaWindowSizeSeconds, remoteLogManagerFetchMaxBytesPerSecond, - remoteLogManagerFetchNumQuotaSamples, remoteLogManagerFetchQuotaWindowSizeSeconds); + protected RemoteLogManagerConfig(Map<?, ?> props, boolean doLog) { Review Comment: > However, the rule is written in 2014 and I don't think "to extend" means "can't compose" I completely agree. There are lots of decisions from 2014/2015 that I disagree with and actively cause me problems, this one fortunately hasn't been one of them. > We don't need to change whole code base when we are aware of good pattern, but it would be nice to apply on the new code. I agree that the two styles can coexist at the same time, and implementing a pattern on "new" code is a good rollout plan. I do think that one should eventually "win", and I was curious if you had a good reason that might make composition win :) -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org