You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/09/07 10:34:12 UTC
[GitHub] [kafka] cadonna commented on a change in pull request #11250: Kafka 12766 - Disabling WAL-related Options in RocksDB
cadonna commented on a change in pull request #11250:
URL: https://github.com/apache/kafka/pull/11250#discussion_r703386486
##########
File path: streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapterTest.java
##########
@@ -126,7 +147,7 @@ private void verifyDBOptionsMethodCall(final Method method) throws Exception {
} catch (final InvocationTargetException undeclaredMockMethodCall) {
assertThat(undeclaredMockMethodCall.getCause(), instanceOf(AssertionError.class));
assertThat(undeclaredMockMethodCall.getCause().getMessage().trim(),
- matchesPattern("Unexpected method call DBOptions\\." + method.getName() + "((.*\n*)*):"));
+ matchesPattern("Unexpected method call DBOptions\\." + method.getName() + "((.*\n*)*):"));
Review comment:
Could you please revert this change since it is unrelated to the PR?
##########
File path: streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapterTest.java
##########
@@ -93,7 +114,7 @@ public void shouldOverwriteAllOptionsMethods() throws Exception {
for (final Method method : Options.class.getMethods()) {
if (!ignoreMethods.contains(method.getName())) {
RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter.class
- .getDeclaredMethod(method.getName(), method.getParameterTypes());
+ .getDeclaredMethod(method.getName(), method.getParameterTypes());
Review comment:
Could you please revert this change since it is unrelated to the PR?
##########
File path: streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapterTest.java
##########
@@ -306,4 +327,33 @@ public String name() {
return parameters;
}
+
+ @Test
+ public void shouldLogWarningWhenSettingWalOptions() throws Exception {
+
+ try (final LogCaptureAppender appender = LogCaptureAppender.createAndRegister(RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter.class)) {
+
+ final RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter adapter
+ = new RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter(new DBOptions(), new ColumnFamilyOptions());
+
+ for (final Method method : RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter.class.getDeclaredMethods()) {
+ if (walRelatedMethods.contains(method.getName())) {
+ method.invoke(adapter, getDBOptionsParameters(method.getParameterTypes()));
+ }
+ }
+
+ final List<String> walOptions = Arrays.asList("walDir", "walFilter", "walRecoveryMode", "walBytesPerSync", "walSizeLimitMB", "manualWalFlush", "maxTotalWalSize", "walTtlSeconds");
Review comment:
nit: Would it be possible to extract this options from `walRelatedMethods`?
##########
File path: streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapterTest.java
##########
@@ -113,7 +134,7 @@ public void shouldForwardAllDbOptionsCalls() throws Exception {
private void verifyDBOptionsMethodCall(final Method method) throws Exception {
final DBOptions mockedDbOptions = mock(DBOptions.class);
final RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter optionsFacadeDbOptions
- = new RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter(mockedDbOptions, new ColumnFamilyOptions());
+ = new RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter(mockedDbOptions, new ColumnFamilyOptions());
Review comment:
Could you please revert this change since it is unrelated to the PR?
##########
File path: streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapterTest.java
##########
@@ -218,7 +239,7 @@ public void shouldForwardAllColumnFamilyCalls() throws Exception {
private void verifyColumnFamilyOptionsMethodCall(final Method method) throws Exception {
final ColumnFamilyOptions mockedColumnFamilyOptions = mock(ColumnFamilyOptions.class);
final RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter optionsFacadeColumnFamilyOptions
- = new RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter(new DBOptions(), mockedColumnFamilyOptions);
+ = new RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter(new DBOptions(), mockedColumnFamilyOptions);
Review comment:
Could you please revert this change since it is unrelated to the PR?
##########
File path: streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapterTest.java
##########
@@ -306,4 +327,33 @@ public String name() {
return parameters;
}
+
+ @Test
+ public void shouldLogWarningWhenSettingWalOptions() throws Exception {
+
+ try (final LogCaptureAppender appender = LogCaptureAppender.createAndRegister(RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter.class)) {
+
+ final RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter adapter
+ = new RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter(new DBOptions(), new ColumnFamilyOptions());
Review comment:
nit:
```suggestion
= new RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter(new DBOptions(), new ColumnFamilyOptions());
```
##########
File path: streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapterTest.java
##########
@@ -231,7 +252,7 @@ private void verifyColumnFamilyOptionsMethodCall(final Method method) throws Exc
} catch (final InvocationTargetException undeclaredMockMethodCall) {
assertThat(undeclaredMockMethodCall.getCause(), instanceOf(AssertionError.class));
assertThat(undeclaredMockMethodCall.getCause().getMessage().trim(),
- matchesPattern("Unexpected method call ColumnFamilyOptions\\." + method.getName() + "(.*)"));
+ matchesPattern("Unexpected method call ColumnFamilyOptions\\." + method.getName() + "(.*)"));
Review comment:
Could you please revert this change since it is unrelated to the PR?
##########
File path: streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapterTest.java
##########
@@ -306,4 +327,33 @@ public String name() {
return parameters;
}
+
+ @Test
+ public void shouldLogWarningWhenSettingWalOptions() throws Exception {
+
+ try (final LogCaptureAppender appender = LogCaptureAppender.createAndRegister(RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter.class)) {
+
+ final RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter adapter
+ = new RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter(new DBOptions(), new ColumnFamilyOptions());
+
+ for (final Method method : RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter.class.getDeclaredMethods()) {
+ if (walRelatedMethods.contains(method.getName())) {
+ method.invoke(adapter, getDBOptionsParameters(method.getParameterTypes()));
+ }
+ }
+
+ final List<String> walOptions = Arrays.asList("walDir", "walFilter", "walRecoveryMode", "walBytesPerSync", "walSizeLimitMB", "manualWalFlush", "maxTotalWalSize", "walTtlSeconds");
+
+ final Set<String> logMessages = appender.getEvents().stream()
+ .filter(e -> e.getLevel().equals("WARN"))
+ .map(LogCaptureAppender.Event::getMessage)
+ .collect(Collectors.toSet());
Review comment:
nit:
```suggestion
final Set<String> logMessages = appender.getEvents().stream()
.filter(e -> e.getLevel().equals("WARN"))
.map(LogCaptureAppender.Event::getMessage)
.collect(Collectors.toSet());
```
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org