You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "bryanck (via GitHub)" <gi...@apache.org> on 2025/03/24 00:38:29 UTC
[PR] Core: Add update event for rewrite manifests [iceberg]
bryanck opened a new pull request, #12627:
URL: https://github.com/apache/iceberg/pull/12627
The rewrite manifests snapshot producer is the only one that does not generate an update event, thus listeners are not notified when a rewrite manifest occurs. This PR adds an update event for rewrite manifests.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Add update event for rewrite manifests [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #12627:
URL: https://github.com/apache/iceberg/pull/12627#discussion_r2009861503
##########
core/src/test/java/org/apache/iceberg/TestCommitReporting.java:
##########
@@ -196,14 +196,9 @@ public void addAndDeleteManifests() throws IOException {
CommitReport report = reporter.lastCommitReport();
assertThat(report).isNotNull();
- assertThat(report.operation()).isEqualTo("append");
- assertThat(report.snapshotId()).isEqualTo(2L);
- assertThat(report.sequenceNumber()).isEqualTo(2L);
+ assertThat(report.operation()).isEqualTo("replace");
+ assertThat(report.snapshotId()).isEqualTo(3L);
+ assertThat(report.sequenceNumber()).isEqualTo(3L);
assertThat(report.tableName()).isEqualTo(tableName);
-
- CommitMetricsResult metrics = report.commitMetrics();
Review Comment:
@bryanck I've opened https://github.com/apache/iceberg/pull/12630 to address ^
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Add update event for rewrite manifests [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #12627:
URL: https://github.com/apache/iceberg/pull/12627#discussion_r2009678199
##########
core/src/test/java/org/apache/iceberg/TestCommitReporting.java:
##########
@@ -196,14 +196,9 @@ public void addAndDeleteManifests() throws IOException {
CommitReport report = reporter.lastCommitReport();
assertThat(report).isNotNull();
- assertThat(report.operation()).isEqualTo("append");
- assertThat(report.snapshotId()).isEqualTo(2L);
- assertThat(report.sequenceNumber()).isEqualTo(2L);
+ assertThat(report.operation()).isEqualTo("replace");
+ assertThat(report.snapshotId()).isEqualTo(3L);
+ assertThat(report.sequenceNumber()).isEqualTo(3L);
assertThat(report.tableName()).isEqualTo(tableName);
-
- CommitMetricsResult metrics = report.commitMetrics();
Review Comment:
I'm fine with the PR as-is. I'll follow up in a separate PR to add proper commit metrics for manifests kept/created/replaced
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Add update event for rewrite manifests [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #12627:
URL: https://github.com/apache/iceberg/pull/12627#discussion_r2009657858
##########
core/src/test/java/org/apache/iceberg/TestCommitReporting.java:
##########
@@ -196,14 +196,9 @@ public void addAndDeleteManifests() throws IOException {
CommitReport report = reporter.lastCommitReport();
assertThat(report).isNotNull();
- assertThat(report.operation()).isEqualTo("append");
- assertThat(report.snapshotId()).isEqualTo(2L);
- assertThat(report.sequenceNumber()).isEqualTo(2L);
+ assertThat(report.operation()).isEqualTo("replace");
+ assertThat(report.snapshotId()).isEqualTo(3L);
+ assertThat(report.sequenceNumber()).isEqualTo(3L);
assertThat(report.tableName()).isEqualTo(tableName);
-
- CommitMetricsResult metrics = report.commitMetrics();
Review Comment:
I'd probably change this to
```
CommitMetricsResult metrics = report.commitMetrics();
assertThat(metrics.totalDataFiles().value()).isEqualTo(2L);
assertThat(metrics.totalRecords().value()).isEqualTo(2L);
assertThat(metrics.totalFilesSizeInBytes().value()).isEqualTo(20L);
```
so that we at least check that some metrics were recorded
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Add update event for rewrite manifests [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra merged PR #12627:
URL: https://github.com/apache/iceberg/pull/12627
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org