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.

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

Throw IndexOutOfBoundsException in MemoryInputStream #19813

Merged

Conversation

jkylling
Copy link
Contributor

Description

InputStream.read is expected to throw IndexOutOfBoundException when

off is negative, len is negative, or len is greater than b.length - off

We currently don't test this, and allow using MemoryInputStream.read with a negative offset.

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Nov 18, 2023
@jkylling jkylling added the bug Something isn't working label Nov 18, 2023
Copy link
Member

@electrum electrum left a comment

Choose a reason for hiding this comment

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

It looks like we need to add the same check to HdfsTrinoInputStream

@jkylling jkylling mentioned this pull request Dec 12, 2023
@raunaqmorarka
Copy link
Member

It looks like we need to add the same check to HdfsTrinoInputStream

@jkylling can you please apply this comment and fix test failure here ?

@jkylling jkylling force-pushed the test-IndexOutOfBoundsException branch from f3348d9 to ce5ca91 Compare December 21, 2023 15:54
@jkylling
Copy link
Contributor Author

I'm unable to run the TestHdfsFileSystemLocal locally, but hopefully the change is simple enough to rely on CI. I get the error below locally:

java.io.UncheckedIOException: org.apache.hadoop.ipc.RemoteException(java.io.IOException): File /level0/level1/level2-file could only be written to 0 of the 1 minReplication nodes. There are 1 datanode(s) running and 1 node(s) are excluded in this operation.
	at org.apache.hadoop.hdfs.server.blockmanagement.BlockManager.chooseTarget4NewBlock(BlockManager.java:2121)
	at org.apache.hadoop.hdfs.server.namenode.FSDirWriteFileOp.chooseTargetForNewBlock(FSDirWriteFileOp.java:286)
	at org.apache.hadoop.hdfs.server.namenode.FSNamesystem.getAdditionalBlock(FSNamesystem.java:2706)
	at org.apache.hadoop.hdfs.server.namenode.NameNodeRpcServer.addBlock(NameNodeRpcServer.java:875)
	at org.apache.hadoop.hdfs.protocolPB.ClientNamenodeProtocolServerSideTranslatorPB.addBlock(ClientNamenodeProtocolServerSideTranslatorPB.java:561)
	at org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos$ClientNamenodeProtocol$2.callBlockingMethod(ClientNamenodeProtocolProtos.java)
	at org.apache.hadoop.ipc.ProtobufRpcEngine$Server$ProtoBufRpcInvoker.call(ProtobufRpcEngine.java:524)
	at org.apache.hadoop.ipc.RPC$Server.call(RPC.java:1025)
	at org.apache.hadoop.ipc.Server$RpcCall.run(Server.java:876)
	at org.apache.hadoop.ipc.Server$RpcCall.run(Server.java:822)
	at java.security.AccessController.doPrivileged(Native Method)
	at javax.security.auth.Subject.doAs(Subject.java:422)
	at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1730)
	at org.apache.hadoop.ipc.Server$Handler.run(Server.java:2682)


	at io.trino.filesystem.AbstractTestTrinoFileSystem$TempBlob.createOrOverwrite(AbstractTestTrinoFileSystem.java:1227)
	at io.trino.filesystem.AbstractTestTrinoFileSystem.createBlob(AbstractTestTrinoFileSystem.java:1149)
	at io.trino.filesystem.AbstractTestTrinoFileSystem.testRenameDirectory(AbstractTestTrinoFileSystem.java:1018)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:194)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165)
Caused by: org.apache.hadoop.ipc.RemoteException(java.io.IOException): File /level0/level1/level2-file could only be written to 0 of the 1 minReplication nodes. There are 1 datanode(s) running and 1 node(s) are excluded in this operation.
	at org.apache.hadoop.hdfs.server.blockmanagement.BlockManager.chooseTarget4NewBlock(BlockManager.java:2121)
	at org.apache.hadoop.hdfs.server.namenode.FSDirWriteFileOp.chooseTargetForNewBlock(FSDirWriteFileOp.java:286)
	at org.apache.hadoop.hdfs.server.namenode.FSNamesystem.getAdditionalBlock(FSNamesystem.java:2706)
	at org.apache.hadoop.hdfs.server.namenode.NameNodeRpcServer.addBlock(NameNodeRpcServer.java:875)
	at org.apache.hadoop.hdfs.protocolPB.ClientNamenodeProtocolServerSideTranslatorPB.addBlock(ClientNamenodeProtocolServerSideTranslatorPB.java:561)
	at org.apache.hadoop.hdfs.protocol.proto.ClientNamenodeProtocolProtos$ClientNamenodeProtocol$2.callBlockingMethod(ClientNamenodeProtocolProtos.java)
	at org.apache.hadoop.ipc.ProtobufRpcEngine$Server$ProtoBufRpcInvoker.call(ProtobufRpcEngine.java:524)

@raunaqmorarka raunaqmorarka force-pushed the test-IndexOutOfBoundsException branch from ce5ca91 to 467bcb6 Compare December 22, 2023 06:17
@wendigo
Copy link
Contributor

wendigo commented Dec 22, 2023

@jkylling please shorten a first line of a commit message according to our guidelines. I'll merge this change then

@raunaqmorarka raunaqmorarka force-pushed the test-IndexOutOfBoundsException branch from 467bcb6 to 8cda0ee Compare December 22, 2023 08:11
@raunaqmorarka raunaqmorarka merged commit 1236521 into trinodb:master Dec 22, 2023
2 of 13 checks passed
@github-actions github-actions bot added this to the 436 milestone Dec 22, 2023
@wendigo
Copy link
Contributor

wendigo commented Dec 22, 2023

Thanks @raunaqmorarka !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cla-signed
Development

Successfully merging this pull request may close these issues.

4 participants