You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "ctubbsii (via GitHub)" <gi...@apache.org> on 2023/09/01 06:00:07 UTC
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #3737: Use custom Transport Factory to set Transport message and frame size
ctubbsii commented on code in PR #3737:
URL: https://github.com/apache/accumulo/pull/3737#discussion_r1312513764
##########
core/src/test/java/org/apache/accumulo/core/rpc/ThriftUtilTest.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.rpc;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.nio.ByteBuffer;
+import java.util.Arrays;
+
+import org.apache.thrift.transport.TByteBuffer;
+import org.apache.thrift.transport.TTransport;
+import org.apache.thrift.transport.TTransportException;
+import org.apache.thrift.transport.layered.TFramedTransport;
+import org.junit.jupiter.api.Test;
+
+public class ThriftUtilTest {
+
+ public static final int FRAME_HDR_SIZE = 4;
+ public static final int MB1 = 1 * 1024 * 1024;
+ public static final int MB10 = 10 * 1024 * 1024;
+ public static final int MB100 = 100 * 1024 * 1024;
+ public static final int GB = 1 * 1024 * 1024 * 1024;
+
+ @Test
+ public void testDefaultTFramedTransportFactory() throws TTransportException {
+
+ // This test confirms that the default maxMessageSize in Thrift is 100MB
+ // even when we set the frame size to be 1GB
+
+ TByteBuffer underlyingTransport = new TByteBuffer(ByteBuffer.allocate(1024));
+
+ TFramedTransport.Factory factory = new TFramedTransport.Factory(GB);
+ TTransport framedTransport = factory.getTransport(underlyingTransport);
+
+ assertEquals(framedTransport.getConfiguration().getMaxFrameSize(), GB);
+ assertEquals(framedTransport.getConfiguration().getMaxMessageSize(), MB100);
+ }
+
+ @Test
+ public void testAccumuloTFramedTransportFactory() throws TTransportException {
+
+ // This test confirms that our custom FramedTransportFactory sets the max
+ // message size and max frame size to the value that we want.
+
+ TByteBuffer underlyingTransport = new TByteBuffer(ByteBuffer.allocate(1024));
+
+ AccumuloTFramedTransportFactory factory = new AccumuloTFramedTransportFactory(GB);
+ TTransport framedTransport = factory.getTransport(underlyingTransport);
+
+ assertEquals(framedTransport.getConfiguration().getMaxFrameSize(), GB);
+ assertEquals(framedTransport.getConfiguration().getMaxMessageSize(), GB);
+ }
+
+ @Test
+ public void testMessageSizeReadWriteSuccess() throws Exception {
+
+ // This test creates an 10MB buffer in memory as the underlying transport, then
+ // creates a TFramedTransport with a 1MB maxFrameSize and maxMessageSize. It then
+ // writes 1MB - 4 bytes (to account for the frame header) to the transport and
+ // reads the data back out.
+
+ TByteBuffer underlyingTransport = new TByteBuffer(ByteBuffer.allocate(MB10));
+ AccumuloTFramedTransportFactory factory = new AccumuloTFramedTransportFactory(MB1);
+ TTransport framedTransport = factory.getTransport(underlyingTransport);
+ assertEquals(framedTransport.getConfiguration().getMaxFrameSize(), MB1);
+ assertEquals(framedTransport.getConfiguration().getMaxMessageSize(), MB1);
+
+ byte[] writeBuf = new byte[MB1 - FRAME_HDR_SIZE];
+ Arrays.fill(writeBuf, (byte) 1);
+ framedTransport.write(writeBuf);
+ framedTransport.flush();
+
+ assertEquals(MB1, underlyingTransport.getByteBuffer().position());
+ underlyingTransport.flip();
+ assertEquals(0, underlyingTransport.getByteBuffer().position());
+ assertEquals(MB1, underlyingTransport.getByteBuffer().limit());
+
+ byte[] readBuf = new byte[MB1];
+ framedTransport.read(readBuf, 0, MB1);
+ }
+
+ @Test
+ public void testMessageSizeWriteFailure() throws Exception {
+
+ // This test creates an 10MB buffer in memory as the underlying transport, then
+ // creates a TFramedTransport with a 1MB maxFrameSize and maxMessageSize. It then
+ // writes 1MB + 100 bytes to the transport, which fails as it's larger than the
+ // configured frame and message size.
+
+ TByteBuffer underlyingTransport = new TByteBuffer(ByteBuffer.allocate(MB10));
+ AccumuloTFramedTransportFactory factory = new AccumuloTFramedTransportFactory(MB1);
+ TTransport framedTransport = factory.getTransport(underlyingTransport);
+ assertEquals(framedTransport.getConfiguration().getMaxFrameSize(), MB1);
+ assertEquals(framedTransport.getConfiguration().getMaxMessageSize(), MB1);
+
+ // Write more than 1MB to the TByteBuffer, it's possible to write more data
+ // than allowed by the frame, it's enforced on the read.
+ byte[] writeBuf = new byte[MB1 + 100];
+ Arrays.fill(writeBuf, (byte) 1);
+ framedTransport.write(writeBuf);
+ framedTransport.flush();
+
+ assertEquals(MB1 + 100 + FRAME_HDR_SIZE, underlyingTransport.getByteBuffer().position());
+ underlyingTransport.flip();
+ assertEquals(0, underlyingTransport.getByteBuffer().position());
+ assertEquals(MB1 + 100 + FRAME_HDR_SIZE, underlyingTransport.getByteBuffer().limit());
+
+ byte[] readBuf = new byte[MB1 + 100];
+ assertThrows(TTransportException.class, () -> framedTransport.read(readBuf, 0, MB1 + 100));
Review Comment:
```suggestion
var e =
assertThrows(TTransportException.class, () -> framedTransport.read(readBuf, 0, MB1 + 100));
assertEquals("Frame size (1048676) larger than max length (1048576)!", e.getMessage());
```
##########
core/src/main/java/org/apache/accumulo/core/rpc/AccumuloTFramedTransportFactory.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.rpc;
+
+import org.apache.thrift.transport.TTransport;
+import org.apache.thrift.transport.TTransportException;
+import org.apache.thrift.transport.layered.TFramedTransport;
+
+public class AccumuloTFramedTransportFactory extends TFramedTransport.Factory {
+
+ private final int maxMessageSize;
+
+ public AccumuloTFramedTransportFactory(int maxMessageSize) {
+ super(maxMessageSize);
+ this.maxMessageSize = maxMessageSize;
+ }
+
+ @Override
+ public TTransport getTransport(TTransport base) throws TTransportException {
+ // The input parameter "base" is typically going to be a TSocket implementation
+ // that represents a connection between two Accumulo endpoints (client-server,
+ // or server-server). The base transport has a maxMessageSize which defaults to
+ // 100MB. The FramedTransport that is created by this factory adds a header to
+ // the message with payload size information. The FramedTransport has a default
+ // frame size of 16MB, but the TFramedTransport constructor sets the frame size
+ // to the frame size set on the underlying transport ("base" in this case").
+ // According to current Thrift docs, a message has to fit into 1 frame, so the
+ // frame size will be set to the value that is lower. Prior to this class being
+ // created, we were only setting the frame size, so messages were capped at 100MB
+ // because that's the default maxMessageSize. Here we are setting the maxMessageSize
+ // and maxFrameSize to the same value on the "base" transport so that when the
+ // TFramedTransport object is created, it ends up using the values that we want.
+ base.getConfiguration().setMaxFrameSize(maxMessageSize);
+ base.getConfiguration().setMaxMessageSize(maxMessageSize);
Review Comment:
I spent some time looking at this, and I think it's a good workaround. However, I think the bug should be fixed upstream. I created https://issues.apache.org/jira/browse/THRIFT-5732
We can leave a note in this class that it can be removed once that issue is fixed and we move to a release with the fix included.
##########
test/src/main/java/org/apache/accumulo/test/functional/ThriftMaxFrameSizeIT.java:
##########
@@ -32,19 +37,20 @@
import org.apache.thrift.TConfiguration;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
+import org.opentest4j.AssertionFailedError;
public class ThriftMaxFrameSizeIT extends AccumuloClusterHarness {
private ThriftServerType serverType;
- @Override
- protected Duration defaultTimeout() {
- return Duration.ofMinutes(1);
- }
-
Review Comment:
I still think we should have a timeout on the ITs.
##########
test/src/main/java/org/apache/accumulo/test/functional/ThriftMaxFrameSizeIT.java:
##########
@@ -56,50 +62,95 @@ class TestDefault extends TestMaxFrameSize {
TestDefault() {
serverType = ThriftServerType.getDefault();
}
+
+ @Test
+ public void testDefaultServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
}
@Nested
class TestThreadedSelector extends TestMaxFrameSize {
TestThreadedSelector() {
serverType = ThriftServerType.THREADED_SELECTOR;
}
+
+ @Test
+ public void testThreadedSelectorServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
}
@Nested
class TestCustomHsHa extends TestMaxFrameSize {
TestCustomHsHa() {
serverType = ThriftServerType.CUSTOM_HS_HA;
}
+
+ @Test
+ public void testCustomHsHaServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
}
@Nested
class TestThreadPool extends TestMaxFrameSize {
TestThreadPool() {
serverType = ThriftServerType.THREADPOOL;
}
+
+ @Test
+ public void testThreadPoolServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
}
@Nested
class TestSsl extends TestMaxFrameSize {
TestSsl() {
- serverType = ThriftServerType.THREADPOOL;
+ serverType = ThriftServerType.SSL;
+ }
+
+ @Test
+ public void testSslServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ testMaxFrameSizeLargerThanDefault(succeeded);
+ assertTrue(succeeded.get());
}
}
protected abstract class TestMaxFrameSize {
- @Test
- public void testMaxFrameSizeLargerThanDefault() throws Exception {
+ public void testMaxFrameSizeLargerThanDefault(AtomicBoolean success) throws Exception {
+
+ int maxSize = TConfiguration.DEFAULT_MAX_FRAME_SIZE;
+ // make sure we go even bigger than that
+ int ourSize = maxSize * 2;
// Ingest with a value width greater than the thrift default size to verify our setting works
- // for max frame wize
+ // for max frame size
try (AccumuloClient accumuloClient = Accumulo.newClient().from(getClientProps()).build()) {
- String table = getUniqueNames(1)[0];
- ReadWriteIT.ingest(accumuloClient, 1, 1, TConfiguration.DEFAULT_MAX_FRAME_SIZE + 1, 0,
- table);
- ReadWriteIT.verify(accumuloClient, 1, 1, TConfiguration.DEFAULT_MAX_FRAME_SIZE + 1, 0,
- table);
+ String table = getUniqueNames(1)[0] + serverType.name();
Review Comment:
I am not sure why this was modified to add the server type name to the table name. If the `@Nested` naming is working correctly, these already have unique names per test.
##########
test/src/main/java/org/apache/accumulo/test/functional/ThriftMaxFrameSizeIT.java:
##########
@@ -56,50 +62,95 @@ class TestDefault extends TestMaxFrameSize {
TestDefault() {
serverType = ThriftServerType.getDefault();
}
+
+ @Test
+ public void testDefaultServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
}
@Nested
class TestThreadedSelector extends TestMaxFrameSize {
TestThreadedSelector() {
serverType = ThriftServerType.THREADED_SELECTOR;
}
+
+ @Test
+ public void testThreadedSelectorServerFrameSize() throws Exception {
Review Comment:
Making these separate tests in each separate `@Nested` class starts defeating the reason these are nested. Plus, all of them except the Ssl one (which I don't understand why it works) has the same implementation. So it can just stay in the base class. The Ssl one could just be made to use a different base class that doesn't have the same method.
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java:
##########
@@ -407,7 +408,8 @@ public void run() {
failures.putAll(tsFailures);
}
}
-
+ } catch (EOFException e) {
+ fatalException = e;
Review Comment:
There's another place, in MetadataLocationObtainer, where this can propagate up, that isn't being handled as a fatal exception. I'm not sure it matters there, though, because it doesn't look like it's retrying there.
##########
test/src/main/java/org/apache/accumulo/test/functional/ThriftMaxFrameSizeIT.java:
##########
@@ -56,50 +62,95 @@ class TestDefault extends TestMaxFrameSize {
TestDefault() {
serverType = ThriftServerType.getDefault();
}
+
+ @Test
+ public void testDefaultServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
Review Comment:
The succeeded boolean is only ever false when the method that sets it is interrupted... which also would throw the assertion failure. So, adding that boolean doesn't actually add more information... it's always false on the interrupt case, and it's always true on the non-interrupt case. So, it's just a proxy for whether or not we used the assertTimeoutPreemptively method. It can be removed.
##########
test/src/main/java/org/apache/accumulo/test/functional/ThriftMaxFrameSizeIT.java:
##########
@@ -32,19 +37,20 @@
import org.apache.thrift.TConfiguration;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
+import org.opentest4j.AssertionFailedError;
public class ThriftMaxFrameSizeIT extends AccumuloClusterHarness {
private ThriftServerType serverType;
- @Override
- protected Duration defaultTimeout() {
- return Duration.ofMinutes(1);
- }
-
@Override
public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration hadoopCoreSite) {
+ cfg.setNumTservers(1);
cfg.setProperty(Property.GENERAL_RPC_SERVER_TYPE, serverType.name());
+ cfg.setProperty(Property.GENERAL_MAX_MESSAGE_SIZE,
+ Integer.toString(TConfiguration.DEFAULT_MAX_FRAME_SIZE));
+ cfg.setProperty(Property.TSERV_MAX_MESSAGE_SIZE,
+ Integer.toString(TConfiguration.DEFAULT_MAX_FRAME_SIZE));
Review Comment:
So, you're fundamentally altering the purpose of this test.
The purpose of this test was to make sure that we could pass messages larger than the 16MB max frame size that was the default. We are inherently relying on the fact that our default values for these on the server side are greater than this max, and we are passing messages that exceed the 16MB, to make sure that the configuration on the server-side worked as expected.
Your changes, however, make it so the server side is configured back to the default, making it indistinguishable from our server-side configuration being ignored, and verifying that the client hangs (except for SSL, which is weird), because it can *not* pass messages larger than the 16MB.
I don't think the changes to this IT are good here.
The situation I ran into when I was looking at this IT, was that I didn't just want to try to pass messages larger than 16MB... I wanted to try to pass messages larger than 100MB, to make sure that the changes in this PR correctly set the max message size when it set the max frame size. I was unable to modify the test to do that, because 100MB is huge, and I kept getting out of memory errors.
Rather than test that we can set it larger than 100MB, we can just test that we're overriding the default of 100MB to something like 20MB. We can then test that we can pass messages over 16MB, but less than 20MB, and we can verify that messages larger than 20MB won't work. But that doesn't tell us anything about whether they failed because they hit the max frame size or because they hit the max message size.
We also still might not be able to easily distinguish in the IT the reason why messages larger than 20MB won't work... it could be because of an OutOfMemoryError on the server, rather than exceeding the max message size. What I found interesting in these changes you made, was that you were basically testing that the client hung, and not that the client failed. Based on the EOFException change in TabletServerBatchReaderIterator, I had expected to see a hard failure, rather than a hang due to retries, but I guess we're hitting this max message issue in more places than that iterator and that change was a very specific change to address a related bug, rather than a general fix for retrying Thrift messages that are too big at the RPC layer.
##########
test/pom.xml:
##########
@@ -206,6 +206,11 @@
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.opentest4j</groupId>
+ <artifactId>opentest4j</artifactId>
+ <version>1.2.0</version>
+ </dependency>
Review Comment:
You don't need this. You can just expect AssertionError, it's parent type.
##########
test/src/main/java/org/apache/accumulo/test/functional/ThriftMaxFrameSizeIT.java:
##########
@@ -56,50 +62,95 @@ class TestDefault extends TestMaxFrameSize {
TestDefault() {
serverType = ThriftServerType.getDefault();
}
+
+ @Test
+ public void testDefaultServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
}
@Nested
class TestThreadedSelector extends TestMaxFrameSize {
TestThreadedSelector() {
serverType = ThriftServerType.THREADED_SELECTOR;
}
+
+ @Test
+ public void testThreadedSelectorServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
}
@Nested
class TestCustomHsHa extends TestMaxFrameSize {
TestCustomHsHa() {
serverType = ThriftServerType.CUSTOM_HS_HA;
}
+
+ @Test
+ public void testCustomHsHaServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
}
@Nested
class TestThreadPool extends TestMaxFrameSize {
TestThreadPool() {
serverType = ThriftServerType.THREADPOOL;
}
+
+ @Test
+ public void testThreadPoolServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
}
@Nested
class TestSsl extends TestMaxFrameSize {
TestSsl() {
- serverType = ThriftServerType.THREADPOOL;
+ serverType = ThriftServerType.SSL;
+ }
+
+ @Test
+ public void testSslServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ testMaxFrameSizeLargerThanDefault(succeeded);
+ assertTrue(succeeded.get());
Review Comment:
I'm really not sure why this is still working. I guess Ssl transports don't care about the max message size for some reason? That's weird.
##########
test/src/main/java/org/apache/accumulo/test/functional/ThriftMaxFrameSizeIT.java:
##########
@@ -56,50 +62,95 @@ class TestDefault extends TestMaxFrameSize {
TestDefault() {
serverType = ThriftServerType.getDefault();
}
+
+ @Test
+ public void testDefaultServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
}
@Nested
class TestThreadedSelector extends TestMaxFrameSize {
TestThreadedSelector() {
serverType = ThriftServerType.THREADED_SELECTOR;
}
+
+ @Test
+ public void testThreadedSelectorServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
}
@Nested
class TestCustomHsHa extends TestMaxFrameSize {
TestCustomHsHa() {
serverType = ThriftServerType.CUSTOM_HS_HA;
}
+
+ @Test
+ public void testCustomHsHaServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
}
@Nested
class TestThreadPool extends TestMaxFrameSize {
TestThreadPool() {
serverType = ThriftServerType.THREADPOOL;
}
+
+ @Test
+ public void testThreadPoolServerFrameSize() throws Exception {
+ AtomicBoolean succeeded = new AtomicBoolean(false);
+ assertThrows(AssertionFailedError.class,
+ () -> assertTimeoutPreemptively(Duration.ofSeconds(30),
+ () -> testMaxFrameSizeLargerThanDefault(succeeded)));
+ assertFalse(succeeded.get());
+ }
}
@Nested
class TestSsl extends TestMaxFrameSize {
TestSsl() {
- serverType = ThriftServerType.THREADPOOL;
+ serverType = ThriftServerType.SSL;
Review Comment:
Well, that one was an interesting typo. We'll want to keep that fix.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org