You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "sadanand48 (via GitHub)" <gi...@apache.org> on 2024/09/05 10:00:34 UTC
[PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
sadanand48 opened a new pull request, #7164:
URL: https://github.com/apache/ozone/pull/7164
## What changes were proposed in this pull request?
Currently this command does not work as on demand changing of replication is not supported in Ozone.
However with the new atomic rewriteKey API , this makes it possible to rewrite key with the new replication and setrep can be implemented using this. This is only for RATIS keys and does not apply for EC keys as there is no point of replication factor in EC keys.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11405
## How was this patch tested?
Unit tests added. Also tested the `ozone fs` shell
```bash
bash-4.4$ ozone sh key info vol1/buck1/key1 | grep "replicationFactor"
"replicationFactor" : "ONE",
bash-4.4$ ozone fs -ls ofs://om/vol1/buck1/key1
-rw-rw-rw- 1 hadoop hadoop 4068 2024-09-05 08:36 ofs://om/vol1/buck1/key1
bash-4.4$ ozone fs -setrep -w 3 ofs://om/vol1/buck1/key1
Replication 3 set: ofs://om/vol1/buck1/key1
Waiting for ofs://om/vol1/buck1/key1 ... done
bash-4.4$ ozone sh key info vol1/buck1/key1 | grep "replicationFactor"
replicationFactor : THREE,
bash-4.4$ ozone fs -setrep -w 2 ofs://om/vol1/buck1/key1
setrep: Replication factor of 2 not supported
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on PR #7164:
URL: https://github.com/apache/ozone/pull/7164#issuecomment-2436930711
For example, the mapreduce.robot file that this patch touches when executed calls [this code](https://github.com/apache/hadoop/blob/d1ce9656459007b5c7f386fdafb018ca16ddf0c9/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/JobResourceUploader.java#L703) and it makes a call to fs.setReplication()
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on PR #7164:
URL: https://github.com/apache/ozone/pull/7164#issuecomment-2421936723
> On the other hand, it looks like -setrep is async by default
Yes it is async in hadoop as the blocks are replicated/deleted according to the set replication factor (client only sends setrep request to NN) , Using rewrite is a hack here and if needed we could do it in a separate thread in order to be asynchronous but it is the client who will do the work of replicating not the server like HDFS.
If this is not desired, we could probably close this and leave the current behaviour as is.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7164:
URL: https://github.com/apache/ozone/pull/7164#issuecomment-2421654137
> Does `ozone fs -setrep` work recursively on vol/bucket/dir at the moment? If not, do we have plans to support it later?
Recursive operation is handled in `SetReplication` (the implementation of `-setrep`), so we get that for free. `FileSystem.setReplication`, which is implemented here for Ozone, only handles files.
On the other hand, it looks like `-setrep` is async by default (`-w` makes it wait for replication to complete), which this implementation does not support. I guess that's part of what @jojochuang referred to here:
> requires client to read from source and write to the destination. I think that wouldn't be expected for a user coming from Hadoop land
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7164:
URL: https://github.com/apache/ozone/pull/7164#issuecomment-2485547523
Thanks again @sadanand48 for the patch. Given that:
- the same functionality is available via `ozone sh` CLI, which even supports rewriting EC keys
- usage via FileSystem API has different behavior compared to Hadoop's async implementation
I suggest abandoning this.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7164:
URL: https://github.com/apache/ozone/pull/7164#issuecomment-2435769514
> Using rewrite is a hack here and if needed we could do it in a separate thread in order to be asynchronous but it is the client who will do the work of replicating not the server like HDFS. If this is not desired, we could probably close this and leave the current behaviour as is.
Alternatively, we could override the shell command from Hadoop, rejecting invocation without `-w` as "not implemented". This would make behavior consistent (since `-w` is forced), and let us add async implementation in the future. (Let me know if more details are needed.)
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on PR #7164:
URL: https://github.com/apache/ozone/pull/7164#issuecomment-2485825158
Sure, closing this.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on PR #7164:
URL: https://github.com/apache/ozone/pull/7164#issuecomment-2436901439
> we could override the shell command from Hadoop, rejecting invocation without -w as "not implemented". This would make behavior consistent (since -w is forced), and let us add async implementation in the future.
This is at the command level i.e only would make sense if user is using ozone fs shell (also won't work for hadoop fs shell as we are overriding in ozone) but at the API level it is still same i.e non-async. Hadoop usecases generally involve using the hadoop fs provided API's and not FS shell , if it is okay there then we could go ahead with this patch.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on code in PR #7164:
URL: https://github.com/apache/ozone/pull/7164#discussion_r1750384310
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java:
##########
@@ -811,4 +817,36 @@ public boolean setSafeMode(SafeModeAction action, boolean isChecked)
return ozoneClient.getProxy().getOzoneManagerClient().setSafeMode(
action, isChecked);
}
+
+ @Override
+ public void setReplication(String key, short replication) throws IOException {
+ OzoneKeyDetails keyDetails = null;
Review Comment:
Done.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractOzoneFileSystemTest.java:
##########
@@ -2289,4 +2293,30 @@ void testSnapshotRead() throws Exception {
assertEquals(2, f2.length);
assertEquals(3, f3.length);
}
+
+ @Test
+ public void testSetReplication() throws IOException {
+ String testKeyName = "testKey" + RandomStringUtils.randomNumeric(4);
+ Path testKeyPath = new Path("/" + testKeyName);
+ try (OzoneOutputStream outputStream = client.getObjectStore()
+ .getVolume(volumeName).getBucket(bucketName)
+ .createKey(testKeyName, 1,
+ RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.ONE),
+ new HashMap<>())) {
+ outputStream.write(RandomUtils.nextBytes(1));
+ }
+ OzoneKeyDetails key = getKey(testKeyPath, false);
+ assertEquals(HddsProtos.ReplicationType.RATIS,
+ key.getReplicationConfig().getReplicationType());
+ assertEquals(ReplicationFactor.ONE.toString(),
+ key.getReplicationConfig().getReplication());
+ o3fs.setReplication(testKeyPath, (short) 3);
+ key = getKey(testKeyPath, false);
+ assertEquals(ReplicationFactor.THREE.toString(),
+ key.getReplicationConfig().getReplication());
+ IOException exception = assertThrows(IOException.class,
+ () -> o3fs.setReplication(testKeyPath, (short) 5),
+ "Does not throw IOException");
+ assertTrue(exception.getMessage().contains("not supported"));
Review Comment:
Done.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on code in PR #7164:
URL: https://github.com/apache/ozone/pull/7164#discussion_r1750384849
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java:
##########
@@ -2610,4 +2611,35 @@ private void deleteFiles(Path base, List<String> fileNames) throws IOException {
}
}
+ @Test
+ public void testSetReplication() throws IOException {
Review Comment:
Done.
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -392,6 +397,41 @@ protected void incrementCounter(Statistic objectsRead, long count) {
//noop: Use RootedOzoneClientAdapterImpl which supports statistics.
}
+ @Override
+ public void setReplication(String key, short replication) throws IOException {
Review Comment:
Done.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 closed pull request #7164: HDDS-11405. Implement setrep command for Ozone FS.
URL: https://github.com/apache/ozone/pull/7164
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7164:
URL: https://github.com/apache/ozone/pull/7164#issuecomment-2436907091
> Hadoop usecases generally involve using the hadoop fs provided API's and not FS shell
Thanks, I didn't know that.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #7164:
URL: https://github.com/apache/ozone/pull/7164#discussion_r1749989258
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java:
##########
@@ -2610,4 +2611,35 @@ private void deleteFiles(Path base, List<String> fileNames) throws IOException {
}
}
+ @Test
+ public void testSetReplication() throws IOException {
Review Comment:
Would it be possible to implement (most of) the new test in `OzoneFileSystemTests` and reuse it in `AbstractOzoneFileSystemTest` and `AbstractRootOzoneFileSystemTest`?
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java:
##########
@@ -811,4 +817,36 @@ public boolean setSafeMode(SafeModeAction action, boolean isChecked)
return ozoneClient.getProxy().getOzoneManagerClient().setSafeMode(
action, isChecked);
}
+
+ @Override
+ public void setReplication(String key, short replication) throws IOException {
+ OzoneKeyDetails keyDetails = null;
Review Comment:
nit: `= null` is unnecessary.
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -392,6 +397,41 @@ protected void incrementCounter(Statistic objectsRead, long count) {
//noop: Use RootedOzoneClientAdapterImpl which supports statistics.
}
+ @Override
+ public void setReplication(String key, short replication) throws IOException {
Review Comment:
Can we avoid duplication between the two adapter classes?
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractOzoneFileSystemTest.java:
##########
@@ -2289,4 +2293,30 @@ void testSnapshotRead() throws Exception {
assertEquals(2, f2.length);
assertEquals(3, f3.length);
}
+
+ @Test
+ public void testSetReplication() throws IOException {
+ String testKeyName = "testKey" + RandomStringUtils.randomNumeric(4);
+ Path testKeyPath = new Path("/" + testKeyName);
+ try (OzoneOutputStream outputStream = client.getObjectStore()
+ .getVolume(volumeName).getBucket(bucketName)
+ .createKey(testKeyName, 1,
+ RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.ONE),
+ new HashMap<>())) {
+ outputStream.write(RandomUtils.nextBytes(1));
+ }
+ OzoneKeyDetails key = getKey(testKeyPath, false);
+ assertEquals(HddsProtos.ReplicationType.RATIS,
+ key.getReplicationConfig().getReplicationType());
+ assertEquals(ReplicationFactor.ONE.toString(),
+ key.getReplicationConfig().getReplication());
+ o3fs.setReplication(testKeyPath, (short) 3);
+ key = getKey(testKeyPath, false);
+ assertEquals(ReplicationFactor.THREE.toString(),
+ key.getReplicationConfig().getReplication());
+ IOException exception = assertThrows(IOException.class,
+ () -> o3fs.setReplication(testKeyPath, (short) 5),
+ "Does not throw IOException");
+ assertTrue(exception.getMessage().contains("not supported"));
Review Comment:
nit: please use `assertThat` for better failure description
```suggestion
assertThat(exception.getMessage()).contains("not supported");
```
(may need import, too)
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #7164:
URL: https://github.com/apache/ozone/pull/7164#discussion_r1805917753
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneClientUtils.java:
##########
@@ -280,4 +290,53 @@ public static void deleteSnapshot(ObjectStore objectStore,
exception.getMessage());
}
}
+
+ public static void setReplication(OzoneConfiguration conf, OzoneBucket bucket,
+ String keyName, short replication) throws IOException {
+ OzoneKeyDetails keyDetails;
+ try {
+ keyDetails = bucket.getKey(keyName);
+ } catch (OMException ome) {
+ // if key does not exist, do nothing
+ if (ome.getResult() == KEY_NOT_FOUND) {
+ return;
+ }
+ throw ome;
+ }
+ ReplicationConfig newReplication =
+ resolveClientSideReplicationConfig(replication, null,
+ null, conf);
+ if (newReplication == null) {
+ throw new IOException(
+ "Replication factor of " + replication + " not supported");
+ }
+ if (newReplication.getReplication()
+ .equals(keyDetails.getReplicationConfig().getReplication())) {
+ return;
+ }
+ try (InputStream inputStream = readFile(bucket, keyName);
+ OzoneOutputStream outputStream = bucket.rewriteKey(keyName,
+ keyDetails.getDataSize(), keyDetails.getGeneration(),
+ newReplication, keyDetails.getMetadata())) {
+ int chunkSize = (int) conf.getStorageSize(OZONE_SCM_CHUNK_SIZE_KEY,
+ OZONE_SCM_CHUNK_SIZE_DEFAULT, StorageUnit.BYTES);
+ IOUtils.copyBytes(inputStream, outputStream, chunkSize);
+ }
Review Comment:
If `KeyOutputStream` is not closed, the rewrite will not be completed:
```
[ERROR] org.apache.hadoop.fs.ozone.TestOFS.testSetReplication Time elapsed: 0.592 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <THREE> but was: <ONE>
```
@sadanand48 it may be useful to add a negative test like that.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11405. Implement setrep command for Ozone FS. [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #7164:
URL: https://github.com/apache/ozone/pull/7164#issuecomment-2435903956
We need more robot tests
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org