You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "hemantk-12 (via GitHub)" <gi...@apache.org> on 2023/07/13 18:42:44 UTC
[GitHub] [ozone] hemantk-12 opened a new pull request, #5061: HDDS-8961. [Snapshot] Print compaction log dag and create an image
hemantk-12 opened a new pull request, #5061:
URL: https://github.com/apache/ozone/pull/5061
## What changes were proposed in this pull request?
This change is to create a operational tool to create an image for the compaction log DAG at any given point in time.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-8961
## How was this patch tested?
Manually tested it on Docker.
--
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
[GitHub] [ozone] adoroszlai commented on a diff in pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1264527674
##########
hadoop-ozone/dist/src/main/license/jar-report.txt:
##########
@@ -145,13 +146,17 @@ share/ozone/lib/jetty-util-ajax.jar
share/ozone/lib/jetty-util.jar
share/ozone/lib/jetty-webapp.jar
share/ozone/lib/jetty-xml.jar
+share/ozone/lib/jgraph.jar
+share/ozone/lib/jgrapht-core.jar
+share/ozone/lib/jgrapht-ext.jar
+share/ozone/lib/jgraphx.jar
Review Comment:
Please restrict changes in `jar-report.txt` to addition of the new lines, avoid reordering existing lines.
See discussion here: https://github.com/apache/ozone/pull/4985#discussion_r1243165718
--
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
[GitHub] [ozone] adoroszlai commented on a diff in pull request #5061: HDDS-8961. [Snapshot] Print compaction log dag and create an image
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1263083553
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/snapshot/PrintCompactionLogDagHandler.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.shell.snapshot;
+
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.shell.Handler;
+import org.apache.hadoop.ozone.shell.OzoneAddress;
+import picocli.CommandLine;
+
+import java.io.IOException;
+
+/**
+ * Handler to generate image for current compaction DAG in the OM.
+ * ozone sh snapshot print-log-dag.
+ */
+@CommandLine.Command(name = "print-log-dag",
+ aliases = "pld",
+ description = "List snapshots for the buckets.")
+public class PrintCompactionLogDagHandler extends Handler {
+
+ @CommandLine.Option(names = {"-f", "--fileName"},
+ description = "Name of the image file. (optional)")
+ private String fileName;
+
+ @CommandLine.Option(names = {"-gt", "--graphType"},
Review Comment:
In addition, please avoid camelCase, use `--graph-type`, etc.
--
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
[GitHub] [ozone] hemantk-12 commented on pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on PR #5061:
URL: https://github.com/apache/ozone/pull/5061#issuecomment-1638972143
> @hemantk-12 @smengcl @prashantpogde I think this PR was incorrectly merged with some existing problems:
>
> * Does this call need to go to the running OM, or can the compaction dag be read by the `ozone debug` process from disk state if run on the OM host? The later is how most Ozone debug calls work. Having a network call that generates a file on the server is strange, which brings me to my other concerns.
>
> * The PR was merged with an [existing security](https://github.com/apache/ozone/pull/5061#discussion_r1263077404) concern not addressed. This definitely needs to be restricted to admins since it could be invoked indefinitely to fill up the OM OS disk.
>
> * There is no regard for absolute or relative paths in the filename parameter. Either some server side handling or documentation in the help message should either handle this case or explain that the base path is fixed.
>
> * Help message does not specify that the image is created on the server side. Users may think the command is broken.
>
> * Response should include which OM generated the file. It will only exist on the OM who is the leader at the time the command was run.
>
>
> If this command can be made to run locally on an OM host without depending on the OM process then a lot of the above concerns should be addressed. Otherwise I think these will need to be fixed, preferably with high priority (especially the security concern). We have enough [tech debt](https://issues.apache.org/jira/browse/HDDS-8097) around Ozone troubleshooting, let's not add more.
Thanks for the review and suggestion @errose28
The reason I made it a OM call because I needed current state of DAG. You can argue that we can get that from compaction log but I had slight doubt that it won't give you exact picture as in memory.
I think most of the concerns can be resolved if I change OM request to return the DAG as java object and construct the image on client side or use compaction log instead of the way it was done.
Should we revert this PR in meanwhile I make the change to address your concern? Or just create new one with fix?
--
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
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #5061: HDDS-8961. [Snapshot] Print compaction log dag and create an image
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1263947457
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/snapshot/PrintCompactionLogDagHandler.java:
##########
Review Comment:
Thanks @smengcl . Moved it under debug as discussed.
--
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
[GitHub] [ozone] prashantpogde merged pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "prashantpogde (via GitHub)" <gi...@apache.org>.
prashantpogde merged PR #5061:
URL: https://github.com/apache/ozone/pull/5061
--
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
[GitHub] [ozone] smengcl commented on a diff in pull request #5061: HDDS-8961. [Snapshot] Print compaction log dag and create an image
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1263073661
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/snapshot/PrintCompactionLogDagHandler.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.shell.snapshot;
+
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.shell.Handler;
+import org.apache.hadoop.ozone.shell.OzoneAddress;
+import picocli.CommandLine;
+
+import java.io.IOException;
+
+/**
+ * Handler to generate image for current compaction DAG in the OM.
+ * ozone sh snapshot print-log-dag.
+ */
+@CommandLine.Command(name = "print-log-dag",
+ aliases = "pld",
+ description = "List snapshots for the buckets.")
+public class PrintCompactionLogDagHandler extends Handler {
+
+ @CommandLine.Option(names = {"-f", "--fileName"},
+ description = "Name of the image file. (optional)")
+ private String fileName;
+
+ @CommandLine.Option(names = {"-gt", "--graphType"},
Review Comment:
Use `-t`, or `--gt` (two dashes)
```suggestion
@CommandLine.Option(names = {"-t", "--graphType"},
```
--
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
[GitHub] [ozone] errose28 commented on pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #5061:
URL: https://github.com/apache/ozone/pull/5061#issuecomment-1638985814
> The reason I made it a OM call because I needed current state of DAG. You can argue that we can get that from compaction log but I had slight doubt that it won't give you exact picture as in memory.
I'm not currently familiar with the snapshot implementation so this is up to you. As a debug tool it might actually be better to return the state from memory like you suggested since that is what the OM is actually using.
> I think most of the concerns can be resolved if I change OM request to return the DAG as java object and construct the image on client side or use compaction log instead of the way it was done.
Yeah some way to pass the result as either a java/proto object or a stream of bytes that the client writes as an image file sounds good, assuming the sizes of either response will not be prohibitively large. We may need to check which approach is easier to implement and generates the smallest size response. Another option is to return the bytes as an svg instead of png. png is not compressed, jpg would be lossy, but svg should give all the info in the most compact form factor. Gzipping as [svgz](https://docs.fileformat.com/image/svgz/) before sending and then unpacking client side could help reduce size on the wire even more.
> Should we revert this PR in meanwhile I make the change to address your concern? Or just create new one with fix?
If the fix can be completed in a few days, let's just create a new one with the fix. I don't think we should cut a release with these changes as is, but I don't think that is happening from apache master too soon.
--
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
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #5061: HDDS-8961. [Snapshot] Print compaction log dag and create an image
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1263947149
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/snapshot/PrintCompactionLogDagHandler.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.shell.snapshot;
+
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.shell.Handler;
+import org.apache.hadoop.ozone.shell.OzoneAddress;
+import picocli.CommandLine;
+
+import java.io.IOException;
+
+/**
+ * Handler to generate image for current compaction DAG in the OM.
+ * ozone sh snapshot print-log-dag.
+ */
+@CommandLine.Command(name = "print-log-dag",
+ aliases = "pld",
+ description = "List snapshots for the buckets.")
+public class PrintCompactionLogDagHandler extends Handler {
+
+ @CommandLine.Option(names = {"-f", "--fileName"},
+ description = "Name of the image file. (optional)")
+ private String fileName;
+
+ @CommandLine.Option(names = {"-gt", "--graphType"},
Review Comment:
Changed as suggested.
--
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
[GitHub] [ozone] smengcl commented on a diff in pull request #5061: HDDS-8961. [Snapshot] Print compaction log dag and create an image
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1263075625
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/snapshot/PrintCompactionLogDagHandler.java:
##########
Review Comment:
Move this under `debug` subcommand.
--
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
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1264603375
##########
hadoop-ozone/dist/src/main/license/jar-report.txt:
##########
@@ -145,13 +146,17 @@ share/ozone/lib/jetty-util-ajax.jar
share/ozone/lib/jetty-util.jar
share/ozone/lib/jetty-webapp.jar
share/ozone/lib/jetty-xml.jar
+share/ozone/lib/jgraph.jar
+share/ozone/lib/jgrapht-core.jar
+share/ozone/lib/jgrapht-ext.jar
+share/ozone/lib/jgraphx.jar
Review Comment:
Initially I tired to restrict it but it failed because some transitive dependency was not there. So I used the auto generated one.
Updated it and manually added only necessary changes.
Thanks for pointing it out and reference.
--
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
[GitHub] [ozone] smengcl commented on a diff in pull request #5061: HDDS-8961. [Snapshot] Print compaction log dag and create an image
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1263073661
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/snapshot/PrintCompactionLogDagHandler.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.shell.snapshot;
+
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.shell.Handler;
+import org.apache.hadoop.ozone.shell.OzoneAddress;
+import picocli.CommandLine;
+
+import java.io.IOException;
+
+/**
+ * Handler to generate image for current compaction DAG in the OM.
+ * ozone sh snapshot print-log-dag.
+ */
+@CommandLine.Command(name = "print-log-dag",
+ aliases = "pld",
+ description = "List snapshots for the buckets.")
+public class PrintCompactionLogDagHandler extends Handler {
+
+ @CommandLine.Option(names = {"-f", "--fileName"},
+ description = "Name of the image file. (optional)")
+ private String fileName;
+
+ @CommandLine.Option(names = {"-gt", "--graphType"},
Review Comment:
`-t`, or `--gt`
```suggestion
@CommandLine.Option(names = {"-t", "--graphType"},
```
--
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
[GitHub] [ozone] smengcl commented on a diff in pull request #5061: HDDS-8961. [Snapshot] Print compaction log dag and create an image
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1263074763
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -265,11 +266,12 @@ message OMRequest {
optional SnapshotPurgeRequest SnapshotPurgeRequest = 118;
optional RecoverLeaseRequest RecoverLeaseRequest = 119;
- optional SetTimesRequest SetTimesRequest = 120;
+ optional SetTimesRequest SetTimesRequest = 120;
Review Comment:
```suggestion
optional SetTimesRequest SetTimesRequest = 120;
```
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -265,11 +266,12 @@ message OMRequest {
optional SnapshotPurgeRequest SnapshotPurgeRequest = 118;
optional RecoverLeaseRequest RecoverLeaseRequest = 119;
- optional SetTimesRequest SetTimesRequest = 120;
+ optional SetTimesRequest SetTimesRequest = 120;
optional RefetchSecretKeyRequest RefetchSecretKeyRequest = 121;
optional ListSnapshotDiffJobRequest ListSnapshotDiffJobRequest = 122;
- optional CancelSnapshotDiffRequest CancelSnapshotDiffRequest = 123;
+ optional CancelSnapshotDiffRequest CancelSnapshotDiffRequest = 123;
Review Comment:
```suggestion
optional CancelSnapshotDiffRequest CancelSnapshotDiffRequest = 123;
```
--
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
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1264345407
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/CompactionLogDagPrinter.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.debug;
+
+import org.apache.hadoop.hdds.cli.SubcommandWithParent;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneClientException;
+import org.apache.hadoop.ozone.shell.Handler;
+import org.apache.hadoop.ozone.shell.OzoneAddress;
+import org.kohsuke.MetaInfServices;
+import picocli.CommandLine;
+
+import java.io.IOException;
+
+/**
+ * Handler to generate image for current compaction DAG in the OM.
+ * ozone sh snapshot print-log-dag.
+ */
+@CommandLine.Command(
+ name = "print-log-dag",
+ aliases = "pld",
+ description = "List snapshots for the buckets.")
Review Comment:
Thanks for the catch. Updated it.
--
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
[GitHub] [ozone] smengcl commented on a diff in pull request #5061: HDDS-8961. [Snapshot] Print compaction log dag and create an image
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1263074984
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -265,11 +266,12 @@ message OMRequest {
optional SnapshotPurgeRequest SnapshotPurgeRequest = 118;
optional RecoverLeaseRequest RecoverLeaseRequest = 119;
- optional SetTimesRequest SetTimesRequest = 120;
+ optional SetTimesRequest SetTimesRequest = 120;
optional RefetchSecretKeyRequest RefetchSecretKeyRequest = 121;
optional ListSnapshotDiffJobRequest ListSnapshotDiffJobRequest = 122;
- optional CancelSnapshotDiffRequest CancelSnapshotDiffRequest = 123;
+ optional CancelSnapshotDiffRequest CancelSnapshotDiffRequest = 123;
optional SetSafeModeRequest SetSafeModeRequest = 124;
+ optional PrintCompactionLogDagRequest PrintCompactionLogDagRequest = 125;
Review Comment:
```suggestion
optional PrintCompactionLogDagRequest PrintCompactionLogDagRequest = 125;
```
--
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
[GitHub] [ozone] errose28 commented on pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #5061:
URL: https://github.com/apache/ozone/pull/5061#issuecomment-1638781980
@hemantk-12 @smengcl @prashantpogde I think this PR was incorrectly merged with some existing problems:
- Does this call need to go to the running OM, or can the compaction dag be read by the `ozone debug` process from disk state if run on the OM host? The later is how most Ozone debug calls work. Having a network call that generates a file on the server is strange, which brings me to my other concerns.
- The PR was merged with an [existing security](https://github.com/apache/ozone/pull/5061#discussion_r1263077404) concern not addressed. This definitely needs to be restricted to admins since it could be invoked indefinitely to fill up the OM OS disk.
- There is no regard for absolute or relative paths in the filename parameter. Either some server side handling or documentation in the help message should either handle this case or explain that the base path is fixed.
- Help message does not specify that the image is created on the server side. Users may think the command is broken.
- Response should include which OM generated the file. It will only exist on the OM who is the leader at the time the command was run.
If this command can be made to run locally on an OM host without depending on the OM process then a lot of the above concerns should be addressed. Otherwise I think these will need to be fixed, preferably with high priority (especially the security concern). We have enough [tech debt](https://issues.apache.org/jira/browse/HDDS-8097) around Ozone troubleshooting, let's not add more.
--
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
[GitHub] [ozone] hemantk-12 commented on pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on PR #5061:
URL: https://github.com/apache/ozone/pull/5061#issuecomment-1641231314
> @hemantk-12 @smengcl @prashantpogde I think this PR was incorrectly merged with some existing problems:
>
> * Does this call need to go to the running OM, or can the compaction dag be read by the `ozone debug` process from disk state if run on the OM host? The later is how most Ozone debug calls work. Having a network call that generates a file on the server is strange, which brings me to my other concerns.
>
> * The PR was merged with an [existing security](https://github.com/apache/ozone/pull/5061#discussion_r1263077404) concern not addressed. This definitely needs to be restricted to admins since it could be invoked indefinitely to fill up the OM OS disk.
>
> * There is no regard for absolute or relative paths in the filename parameter. Either some server side handling or documentation in the help message should either handle this case or explain that the base path is fixed.
>
> * Help message does not specify that the image is created on the server side. Users may think the command is broken.
>
> * Response should include which OM generated the file. It will only exist on the OM who is the leader at the time the command was run.
>
>
> If this command can be made to run locally on an OM host without depending on the OM process then a lot of the above concerns should be addressed. Otherwise I think these will need to be fixed, preferably with high priority (especially the security concern). We have enough [tech debt](https://issues.apache.org/jira/browse/HDDS-8097) around Ozone troubleshooting, let's not add more.
Raised PR: https://github.com/apache/ozone/pull/5087 to address major concerns.
--
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
[GitHub] [ozone] smengcl commented on a diff in pull request #5061: HDDS-8961. [Snapshot] Print compaction log dag and create an image
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1263077404
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -4648,6 +4650,29 @@ public List<SnapshotDiffJob> listSnapshotDiffJobs(String volume,
bucket, jobStatus, listAll);
}
+ public String printCompactionLogDag(String fileName,
+ String graphType)
+ throws IOException {
+
Review Comment:
Add a check here to allow only Ozone admins to run 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
[GitHub] [ozone] smengcl commented on a diff in pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1267223584
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -4648,6 +4650,29 @@ public List<SnapshotDiffJob> listSnapshotDiffJobs(String volume,
bucket, jobStatus, listAll);
}
+ public String printCompactionLogDag(String fileName,
+ String graphType)
+ throws IOException {
+
Review Comment:
Yes. Otherwise anyone can call this and fill up the `/tmp`
--
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
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1267282970
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -4648,6 +4650,29 @@ public List<SnapshotDiffJob> listSnapshotDiffJobs(String volume,
bucket, jobStatus, listAll);
}
+ public String printCompactionLogDag(String fileName,
+ String graphType)
+ throws IOException {
+
Review Comment:
I'll raise a PR addressing this comment and @errose28's concern.
--
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
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1264603437
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -4648,6 +4650,29 @@ public List<SnapshotDiffJob> listSnapshotDiffJobs(String volume,
bucket, jobStatus, listAll);
}
+ public String printCompactionLogDag(String fileName,
+ String graphType)
+ throws IOException {
+
Review Comment:
Do we need this check explicitly?
--
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
[GitHub] [ozone] prashantpogde commented on pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "prashantpogde (via GitHub)" <gi...@apache.org>.
prashantpogde commented on PR #5061:
URL: https://github.com/apache/ozone/pull/5061#issuecomment-1637002425
Thank you @hemantk-12 for this PR and addressing review comments.
--
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
[GitHub] [ozone] adoroszlai commented on a diff in pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1264072722
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/CompactionLogDagPrinter.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.debug;
+
+import org.apache.hadoop.hdds.cli.SubcommandWithParent;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneClientException;
+import org.apache.hadoop.ozone.shell.Handler;
+import org.apache.hadoop.ozone.shell.OzoneAddress;
+import org.kohsuke.MetaInfServices;
+import picocli.CommandLine;
+
+import java.io.IOException;
+
+/**
+ * Handler to generate image for current compaction DAG in the OM.
+ * ozone sh snapshot print-log-dag.
+ */
+@CommandLine.Command(
+ name = "print-log-dag",
+ aliases = "pld",
+ description = "List snapshots for the buckets.")
Review Comment:
Leftover description.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/CompactionLogDagPrinter.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.debug;
+
+import org.apache.hadoop.hdds.cli.SubcommandWithParent;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneClientException;
+import org.apache.hadoop.ozone.shell.Handler;
+import org.apache.hadoop.ozone.shell.OzoneAddress;
+import org.kohsuke.MetaInfServices;
+import picocli.CommandLine;
+
+import java.io.IOException;
+
+/**
+ * Handler to generate image for current compaction DAG in the OM.
+ * ozone sh snapshot print-log-dag.
+ */
+@CommandLine.Command(
+ name = "print-log-dag",
+ aliases = "pld",
+ description = "List snapshots for the buckets.")
+@MetaInfServices(SubcommandWithParent.class)
+public class CompactionLogDagPrinter extends Handler
+ implements SubcommandWithParent {
+
+ @CommandLine.Option(names = {"-f", "--file-name"},
+ description = "Name of the image file. (optional)")
+ private String fileName;
+
+ @CommandLine.Option(names = {"-t", "--graph-type"},
+ description = "Type of node name to use in the graph image.\n" +
+ "Accepted values are: \n" +
+ " file_name: to use file name as node name in DAG,\n" +
+ " key_size: to show the no. of keys in the file along with file " +
+ "name in the DAG node name,\n" +
+ " cumulative_size: to show the cumulative size along with file " +
+ "name in the DAG node name.",
+ defaultValue = "file_name")
+ private String graphType;
Review Comment:
Possible future improvement: use `enum`.
--
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
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1264345433
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/CompactionLogDagPrinter.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.debug;
+
+import org.apache.hadoop.hdds.cli.SubcommandWithParent;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneClientException;
+import org.apache.hadoop.ozone.shell.Handler;
+import org.apache.hadoop.ozone.shell.OzoneAddress;
+import org.kohsuke.MetaInfServices;
+import picocli.CommandLine;
+
+import java.io.IOException;
+
+/**
+ * Handler to generate image for current compaction DAG in the OM.
+ * ozone sh snapshot print-log-dag.
+ */
+@CommandLine.Command(
+ name = "print-log-dag",
+ aliases = "pld",
+ description = "List snapshots for the buckets.")
+@MetaInfServices(SubcommandWithParent.class)
+public class CompactionLogDagPrinter extends Handler
+ implements SubcommandWithParent {
+
+ @CommandLine.Option(names = {"-f", "--file-name"},
+ description = "Name of the image file. (optional)")
+ private String fileName;
+
+ @CommandLine.Option(names = {"-t", "--graph-type"},
+ description = "Type of node name to use in the graph image.\n" +
+ "Accepted values are: \n" +
+ " file_name: to use file name as node name in DAG,\n" +
+ " key_size: to show the no. of keys in the file along with file " +
+ "name in the DAG node name,\n" +
+ " cumulative_size: to show the cumulative size along with file " +
+ "name in the DAG node name.",
+ defaultValue = "file_name")
+ private String graphType;
Review Comment:
Added a TODO to change it to enum.
--
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
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #5061: HDDS-8961. Write a standalone tool to create a visual graph from compaction log files.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1264603375
##########
hadoop-ozone/dist/src/main/license/jar-report.txt:
##########
@@ -145,13 +146,17 @@ share/ozone/lib/jetty-util-ajax.jar
share/ozone/lib/jetty-util.jar
share/ozone/lib/jetty-webapp.jar
share/ozone/lib/jetty-xml.jar
+share/ozone/lib/jgraph.jar
+share/ozone/lib/jgrapht-core.jar
+share/ozone/lib/jgrapht-ext.jar
+share/ozone/lib/jgraphx.jar
Review Comment:
Initially I tired to restrict it but it failed because some transitive dependency was not there. So I used the auto generated one.
Updated it and manually added only necessary changes.
Thanks for pointing it out.
--
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
[GitHub] [ozone] smengcl commented on a diff in pull request #5061: HDDS-8961. [Snapshot] Print compaction log dag and create an image
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5061:
URL: https://github.com/apache/ozone/pull/5061#discussion_r1263075156
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -380,11 +382,12 @@ message OMResponse {
optional hdds.TransferLeadershipResponseProto TransferOmLeadershipResponse = 117;
optional SnapshotPurgeResponse SnapshotPurgeResponse = 118;
optional RecoverLeaseResponse RecoverLeaseResponse = 119;
- optional SetTimesResponse SetTimesResponse = 120;
+ optional SetTimesResponse SetTimesResponse = 120;
optional RefetchSecretKeyResponse RefetchSecretKeyResponse = 121;
optional ListSnapshotDiffJobResponse ListSnapshotDiffJobResponse = 122;
- optional CancelSnapshotDiffResponse cancelSnapshotDiffResponse = 123;
+ optional CancelSnapshotDiffResponse cancelSnapshotDiffResponse = 123;
optional SetSafeModeResponse SetSafeModeResponse = 124;
+ optional PrintCompactionLogDagResponse PrintCompactionLogDagResponse = 125;
Review Comment:
nit: alignment
--
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