Re: [PR] Add interfaces for Action CheckSnapshotIntegrity [iceberg]
github-actions[bot] closed pull request #10642: Add interfaces for Action CheckSnapshotIntegrity URL: https://github.com/apache/iceberg/pull/10642 -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Add interfaces for Action CheckSnapshotIntegrity [iceberg]
github-actions[bot] commented on PR #10642: URL: https://github.com/apache/iceberg/pull/10642#issuecomment-2480193846 This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Add interfaces for Action CheckSnapshotIntegrity [iceberg]
github-actions[bot] commented on PR #10642: URL: https://github.com/apache/iceberg/pull/10642#issuecomment-2465928725 This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the d...@iceberg.apache.org list. Thank you for your contributions. -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Add interfaces for Action CheckSnapshotIntegrity [iceberg]
szehon-ho commented on PR #10642: URL: https://github.com/apache/iceberg/pull/10642#issuecomment-2229469860 Yea looks like there's some discussion in https://github.com/apache/iceberg/pull/10445 and also https://lists.apache.org/thread/6zvpcyvdzglh800v15mt7m7rt76ch3ol. Initially it makes sense to have a central procedure that can optionally do any of these things. -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Add interfaces for Action CheckSnapshotIntegrity [iceberg]
RussellSpitzer commented on PR #10642: URL: https://github.com/apache/iceberg/pull/10642#issuecomment-2229452334 I think this is one where we need to have a bit more discussion on the API. I think we've talked previously about how this would be helpful to have as a generic "Repair Snapshot" command which has a couple purposes. 1) Can check if all data files and manifests in the Snapshot are present 2) Remove missing data files and manifests if requested 3) Rewrite manifests if metadata in manifests doesn't match what is in file footers Might be good to bring this up on the dev list for discussion -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Add interfaces for Action CheckSnapshotIntegrity [iceberg]
himadripal commented on code in PR #10642:
URL: https://github.com/apache/iceberg/pull/10642#discussion_r1670931734
##
api/src/main/java/org/apache/iceberg/actions/CheckSnapshotIntegrity.java:
##
@@ -0,0 +1,53 @@
+/*
+ * 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.iceberg.actions;
+
+import java.util.concurrent.ExecutorService;
+
+public interface CheckSnapshotIntegrity
+extends Action {
+
+ /**
+ * Passes an alternative executor service that will be used for snapshot
integrity checking. If
+ * this method is not called, snapshot integrity checker will still be
running by a single
+ * threaded executor service.
+ *
+ * @param executorService an executor service to parallelize tasks to check
snapshot integrity
+ * @return this for method chaining
+ */
+ CheckSnapshotIntegrity executeWith(ExecutorService executorService);
+
+ /**
+ * Pass the target version to check. The action checks the snapshots in the
target version, not in
+ * the current version of the table.
+ *
+ * @param targetVersion the target version file to be checked. Either a file
name or a file path
+ * is acceptable. For example, it could be either
+ * "1-8893aa9e-f92e-4443-80e7-cfa42238a654.metadata.json" or
+ * "/path/to/1-8893aa9e-f92e-4443-80e7-cfa42238a654.metadata.json".
+ * @return this for method chaining
+ */
+ CheckSnapshotIntegrity targetVersion(String targetVersion);
Review Comment:
should the param name be changed to `targetVersionFilePointer` or
`targetVersionMetadataFile`
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Add interfaces for Action CheckSnapshotIntegrity [iceberg]
himadripal commented on code in PR #10642:
URL: https://github.com/apache/iceberg/pull/10642#discussion_r1670931734
##
api/src/main/java/org/apache/iceberg/actions/CheckSnapshotIntegrity.java:
##
@@ -0,0 +1,53 @@
+/*
+ * 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.iceberg.actions;
+
+import java.util.concurrent.ExecutorService;
+
+public interface CheckSnapshotIntegrity
+extends Action {
+
+ /**
+ * Passes an alternative executor service that will be used for snapshot
integrity checking. If
+ * this method is not called, snapshot integrity checker will still be
running by a single
+ * threaded executor service.
+ *
+ * @param executorService an executor service to parallelize tasks to check
snapshot integrity
+ * @return this for method chaining
+ */
+ CheckSnapshotIntegrity executeWith(ExecutorService executorService);
+
+ /**
+ * Pass the target version to check. The action checks the snapshots in the
target version, not in
+ * the current version of the table.
+ *
+ * @param targetVersion the target version file to be checked. Either a file
name or a file path
+ * is acceptable. For example, it could be either
+ * "1-8893aa9e-f92e-4443-80e7-cfa42238a654.metadata.json" or
+ * "/path/to/1-8893aa9e-f92e-4443-80e7-cfa42238a654.metadata.json".
+ * @return this for method chaining
+ */
+ CheckSnapshotIntegrity targetVersion(String targetVersion);
Review Comment:
should the param name be changed to `targetVersionFilePointer`
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Add interfaces for Action CheckSnapshotIntegrity [iceberg]
himadripal commented on code in PR #10642:
URL: https://github.com/apache/iceberg/pull/10642#discussion_r1670928016
##
api/src/main/java/org/apache/iceberg/actions/CheckSnapshotIntegrity.java:
##
@@ -0,0 +1,53 @@
+/*
+ * 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.iceberg.actions;
+
+import java.util.concurrent.ExecutorService;
+
+public interface CheckSnapshotIntegrity
+extends Action {
+
+ /**
+ * Passes an alternative executor service that will be used for snapshot
integrity checking. If
+ * this method is not called, snapshot integrity checker will still be
running by a single
Review Comment:
nit: 2nd part can be rewritten as `a single threaded executor service is
used by default`
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
