Re: [PR] HDFS-17462. Fix NPE in Router concat when trg is an empty ile [hadoop]
hadoop-yetus commented on PR #6722: URL: https://github.com/apache/hadoop/pull/6722#issuecomment-2074005099 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| _ Prechecks _ | | +1 :green_heart: | dupname | 0m 00s | | No case conflicting files found. | | +0 :ok: | spotbugs | 0m 00s | | spotbugs executables are not available. | | +0 :ok: | codespell | 0m 00s | | codespell was not available. | | +0 :ok: | detsecrets | 0m 00s | | detect-secrets was not available. | | +1 :green_heart: | @author | 0m 00s | | The patch does not contain any @author tags. | | -1 :x: | test4tests | 0m 00s | | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. | _ trunk Compile Tests _ | | +1 :green_heart: | mvninstall | 87m 10s | | trunk passed | | +1 :green_heart: | compile | 4m 55s | | trunk passed | | +1 :green_heart: | checkstyle | 4m 33s | | trunk passed | | +1 :green_heart: | mvnsite | 4m 53s | | trunk passed | | +1 :green_heart: | javadoc | 4m 31s | | trunk passed | | +1 :green_heart: | shadedclient | 137m 05s | | branch has no errors when building and testing our client artifacts. | _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 3m 03s | | the patch passed | | +1 :green_heart: | compile | 2m 24s | | the patch passed | | +1 :green_heart: | javac | 2m 24s | | the patch passed | | +1 :green_heart: | blanks | 0m 00s | | The patch has no blanks issues. | | +1 :green_heart: | checkstyle | 2m 08s | | the patch passed | | +1 :green_heart: | mvnsite | 2m 27s | | the patch passed | | +1 :green_heart: | javadoc | 2m 16s | | the patch passed | | +1 :green_heart: | shadedclient | 151m 08s | | patch has no errors when building and testing our client artifacts. | _ Other Tests _ | | +1 :green_heart: | asflicense | 5m 26s | | The patch does not generate ASF License warnings. | | | | 398m 52s | | | | Subsystem | Report/Notes | |--:|:-| | GITHUB PR | https://github.com/apache/hadoop/pull/6722 | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets | | uname | MINGW64_NT-10.0-17763 81e6d3c33564 3.4.10-87d57229.x86_64 2024-02-14 20:17 UTC x86_64 Msys | | Build tool | maven | | Personality | /c/hadoop/dev-support/bin/hadoop.sh | | git revision | trunk / cff78d9cb5d00eb38a44ab14958473155a7c31e1 | | Default Java | Azul Systems, Inc.-1.8.0_332-b09 | | Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6722/1/testReport/ | | modules | C: hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project/hadoop-hdfs-rbf | | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6722/1/console | | versions | git=2.44.0.windows.1 | | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org | This message was automatically generated. -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17462. Fix NPE in Router concat when trg is an empty ile [hadoop]
fannaihao commented on PR #6722: URL: https://github.com/apache/hadoop/pull/6722#issuecomment-2054796201 @ayushtkn Thanks for the comment! I will add tests for this. Yes, I also think it's better to have a direct way to get the block pool id of an empty file then verify, but I failed to find such methods.. I checked `getFileInfo`, seems it does not return info about block pool id? If I miss something, please let me know. The `expectedBlockPoolId` thing is somehow indirect, but it works right.. -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[PR] HDFS-17462. Fix NPE in Router concat when trg is an empty ile [hadoop]
fannaihao opened a new pull request, #6722: URL: https://github.com/apache/hadoop/pull/6722 ### Description of PR When trg of Router concat is an empty file, it will trigger NPE in Router, and the concat will fail, example: ![image](https://github.com/apache/hadoop/assets/40593494/4edc0aed-08ee-4e1d-8236-84c20f61d15d) This is because when trg is an empty file, NameNode will return lastLocatedBlock as null in the response of getBlockLocations. And Router will not check null of lastLocatedBlock returned, instead Router will use it to get block pool id directly. Trg of concat is an empty file should be allowed in router since this case is supported by concat of NameNode. This PR fix this NPE exception. ### How was this patch tested? ![image](https://github.com/apache/hadoop/assets/40593494/23a46672-cd3a-4a54-8f4d-9c833b2d560c) ### For code changes: If lastLocatedBlock returned from getBlockLocations is null in Router concat, it will not be used to get block pool id. In this case, the block pool id check of trg will be delayed, i.e., concat continues to get and check block pool id of files in src, and only check them. And the check of trg block pool id can be achieved in following steps, i.e., getLocationForPath and the request of concat forwarded to NameNode. And exceptions will be thrown if block pool id of trg is not match with the block pool id of any file in src. - [ ] Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')? - [ ] Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? - [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files? -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org