adoroszlai commented on a change in pull request #1075:
URL: https://github.com/apache/hadoop-ozone/pull/1075#discussion_r450014240
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VolumeManager.java
##########
@@ -35,25 +35,6 @@
void createVolume(OmVolumeArgs args)
Review comment:
I think it's safe to remove this. `BenchMarkOMKeyAllocation` does not
work anyway:
```
$ bin/ozone genesis -b BenchMarkOMKeyAllocation
...
VOLUME_NOT_FOUND org.apache.hadoop.ozone.om.exceptions.OMException: Volume
doesn't exist
at
org.apache.hadoop.ozone.om.BucketManagerImpl.createBucket(BucketManagerImpl.java:130)
at
org.apache.hadoop.ozone.genesis.BenchMarkOMKeyAllocation.setup(BenchMarkOMKeyAllocation.java:86)
at
org.apache.hadoop.ozone.genesis.generated.BenchMarkOMKeyAllocation_keyCreation_jmhTest._jmh_tryInit_f_benchmarkomkeyallocation0_0(BenchMarkOMKeyAllocation_keyCreation_jmhTest.java:338)
at
org.apache.hadoop.ozone.genesis.generated.BenchMarkOMKeyAllocation_keyCreation_jmhTest.keyCreation_Throughput(BenchMarkOMKeyAllocation_keyCreation_jmhTest.java:71)
```
##########
File path:
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java
##########
@@ -72,7 +73,9 @@
* @param args - Arguments to create Volume.
* @throws IOException
*/
- void createVolume(OmVolumeArgs args) throws IOException;
+ default void createVolume(OmVolumeArgs args) throws IOException {
+ throw new NotImplementedException("createVolume is not implemented");
Review comment:
`createVolume` is called directly from `BenchMarkOzoneManager`:
```
$ bin/ozone genesis -b BenchMarkOzoneManager
...
org.apache.commons.lang3.NotImplementedException: createVolume is not
implemented
at
org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol.createVolume(OzoneManagerProtocol.java:77)
at
org.apache.hadoop.ozone.genesis.BenchMarkOzoneManager.initialize(BenchMarkOzoneManager.java:110)
at
org.apache.hadoop.ozone.genesis.generated.BenchMarkOzoneManager_allocateBlockBenchMark_jmhTest._jmh_tryInit_f_benchmarkozonemanager0_0(BenchMarkOzoneManager_allocateBlockBenchMark_jmhTest.java:351)
at
org.apache.hadoop.ozone.genesis.generated.BenchMarkOzoneManager_allocateBlockBenchMark_jmhTest.allocateBlockBenchMark_Throughput(BenchMarkOzoneManager_allocateBlockBenchMark_jmhTest.java:72)
```
`BenchMarkOzoneManager` no longer works even on `master` (same error as with
`BenchMarkOMKeyAllocation`).
##########
File path:
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java
##########
@@ -82,15 +85,19 @@
* already the owner.
* @throws IOException
*/
- boolean setOwner(String volume, String owner) throws IOException;
+ default boolean setOwner(String volume, String owner) throws IOException {
+ throw new NotImplementedException("setOwner is not implemented");
Review comment:
Instead of changing the interface, I think it would be safer to throw
exception only in `OzoneManager`'s "implementation". That way new implementors
would be required to implement these methods or deliberately decide not to.
If you agree, then please also consider changing to
`UnsupportedOperationException` (as
>
[`NotImplementedException`](https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/NotImplementedException.html)
represents the case where the author has yet to implement the logic at this
point in the program
which would not be the case with `OzoneManager`.
In the long run I think we should split `OzoneManagerProtocol` interface
into separate read and write interfaces. `OzoneManager` should only implement
read interface, and `OzoneManagerProtocolClientSideTranslatorPB` should
implement both.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
