HDDS-4147. Add OFS to FileSystem META-INF #1352
Conversation
Should solely rely on META-INF to get the correct HCFS FileSystem class. Also sets all defaultFS in docker compose to ofs://. (cherry picked from commit 389a45fd600fb620c71daf900be61e058ffe7288)
(cherry picked from commit ba080fc9e6ef4391864417ac1daf0d2953712aae)
elek
left a comment
elek
left a comment
There was a problem hiding this comment.
Thanks the patch @smengcl, looks good to me. Good simplification.
I think the patch contains some unnecessary lines from the original patch which are not related to the change to use META-INF/services discovery instead of configuration.
|
|
||
| CORE-SITE.XML_fs.o3fs.impl=org.apache.hadoop.fs.ozone.OzoneFileSystem | ||
| CORE-SITE.XML_fs.defaultFS=o3fs://bucket1.volume1.omservice | ||
| CORE-SITE.XML_fs.defaultFS=ofs://omservice |
There was a problem hiding this comment.
Why do we need this line to use META-INF services based discovery?
There was a problem hiding this comment.
I think only MR tests (ozone-mr and ozonesecure-mr) use defaultFS when setting up MR environment:
Changing defaultFS to ofs:// in these environments can help validate the META-INF changes.
Changing/adding defaultFS in the other ones is not needed IMO.
There was a problem hiding this comment.
AFAIK these are just helper lines, not really the test of the file system. META-INF/services are already tested as test.sh is modified to do both o3fs/ofs tests.
I am fine to replace all the defaultFS just let's agree first what do we need to test. Do we need different MR test with both kind of defaultFs?
Or is it enough to test o3fs/ofs low level functionality and MR can use just one of them?
(And you see it's a different discussion, that's the reason why I suggested to do it in a separated PR. But can be included if you think it's better.)
There was a problem hiding this comment.
META-INF/services are already tested as test.sh is modified to do both o3fs/ofs tests.
Got it.
Do we need different MR test with both kind of
defaultFs?
I'm doing that in HDDS-3840. I'll pass it via -D command-line option, as different value is needed for each test in the same cluster.
I guess I can retain the cluster-level config for convenience.
This reverts commit 4593e8a. Change-Id: I089c067cfdc450502701557ff174d87861fd8532
Change-Id: I96fde1f02397764866d89d23fd81c47237abf636
|
Thanks @adoroszlai for verifying the changes. @elek I believe we are ready to merge this one? I see +1 from @adoroszlai . I will merge this shortly and post a follow up PR for fs.defaultFS. |
* master: (26 commits) HDDS-4167. Acceptance test logs missing if fails during cluster startup (apache#1366) HDDS-4121. Implement OmMetadataMangerImpl#getExpiredOpenKeys. (apache#1351) HDDS-3867. Extend the chunkinfo tool to display information from all nodes in the pipeline. (apache#1154) HDDS-4077. Incomplete OzoneFileSystem statistics (apache#1329) HDDS-3903. OzoneRpcClient support batch rename keys. (apache#1150) HDDS-4151. Skip the inputstream while offset larger than zero in s3g (apache#1354) HDDS-4147. Add OFS to FileSystem META-INF (apache#1352) HDDS-4137. Turn on the verbose mode of safe mode check on testlib (apache#1343) HDDS-4146. Show the ScmId and ClusterId in the scm web ui. (apache#1350) HDDS-4145. Bump version to 1.1.0-SNAPSHOT on master (apache#1349) HDDS-4109. Tests in TestOzoneFileSystem should use the existing MiniOzoneCluster (apache#1316) HDDS-4149. Implement OzoneFileStatus#toString (apache#1356) HDDS-4153. Increase default timeout in kubernetes tests (apache#1357) HDDS-2411. add a datanode chunk validator fo datanode chunk generator (apache#1312) HDDS-4140. Auto-close /pending pull requests after 21 days of inactivity (apache#1344) HDDS-4152. Archive container logs for kubernetes check (apache#1355) HDDS-4056. Convert OzoneAdmin to pluggable model (apache#1285) HDDS-3972. Add option to limit number of items displaying through ldb tool. (apache#1206) HDDS-4068. Client should not retry same OM on network connection failure (apache#1324) HDDS-4062. Non rack aware pipelines should not be created if multiple racks are alive. (apache#1291) ...
3 participants
What changes were proposed in this pull request?
With this change,
fs.ofs.implwon't have to be explicitly set incore-site.xmlwhen using OFS on a client.This change is derived from #1088
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4147
How was this patch tested?
Existing OFS tests shall pass even with
conf.set("fs.ofs.impl", ...)removed.