rich7420 commented on code in PR #11348:
URL: https://github.com/apache/ozone/pull/11348#discussion_r4182556370
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneAtRestEncryption.java:
##########
@@ -672,6 +673,19 @@ private void testMultipartUploadWithEncryption(OzoneBucket
bucket,
try (OzoneInputStream inputStream = bucket.readKey(keyName)) {
assertInstanceOf(MultipartInputStream.class,
inputStream.getInputStream());
+ if (numParts == 2) {
+ inputStream.seek(123);
+ PositionedReadTestHelper.runConcurrentPositionedReads(inputData,
inputStream::readFully);
Review Comment:
Could we also use this helper in `testPutKeyWithEncryption` to cover
concurrent positioned reads for a regular encrypted key? That path uses
Hadoop's `CryptoInputStream` directly, so it would be useful to verify the data
and unchanged sequential position through that wrapper as well.
##########
hadoop-hdds/client/src/test/java/org/apache/hadoop/ozone/client/io/TestECBlockInputStreamProxy.java:
##########
@@ -328,6 +341,95 @@ public void testCanSeekToNewPosition() throws IOException {
}
}
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void emptyPositionedReadsDoNotCreateReaders(boolean missing) throws
Exception {
+ int length = 100;
+ generateData(length);
+ BlockLocationInfo info = ECStreamTestUtil.createKeyInfo(repConfig, length,
+ missing ? ECStreamTestUtil.createIndexMap(2, 3, 4, 5) :
ECStreamTestUtil.createIndexMap(1, 2, 3, 4, 5));
+ try (ECBlockInputStreamProxy stream = createBISProxy(repConfig, info)) {
+ BlockExtendedInputStream reader =
streamFactory.getStreams().get(missing);
+ ByteBuffer empty = ByteBuffer.allocate(0);
+ for (long position : new long[] {0, length, length + 1}) {
+ assertEquals(0, stream.readPositioned(position, empty));
+ assertEquals(0, stream.read(position, empty));
+ }
+ assertThat(streamFactory.getStreams().get(missing)).isSameAs(reader);
+ assertEquals(-1, stream.readPositioned(length, ByteBuffer.allocate(1)));
+ assertEquals(-1, stream.readPositioned(length + 1,
ByteBuffer.allocate(1)));
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"healthy", "missing", "failover", "failure"})
+ @Timeout(30)
Review Comment:
nit: Please remove the new per-test `@Timeout` annotations, following
HDDS-12575. The barriers and futures already have explicit timeouts.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
