You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gobblin.apache.org by "agam-99 (via GitHub)" <gi...@apache.org> on 2026/10/13 05:36:59 UTC
[PR] [GOBBLIN-XXXX] Add retention (deletion) counts to DatasetMetric in GaaSJobObservabilityEvent [gobblin]
agam-99 opened a new pull request, #4209:
URL: https://github.com/apache/gobblin/pull/4209
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
### JIRA
- [ ] My PR addresses the following [Gobblin JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references them in the PR title:
- https://issues.apache.org/jira/browse/GOBBLIN-XXXX
### Description
- [x] Here are some details about my PR, including screenshots (if applicable):
`GaaSJobObservabilityEvent.datasetsMetrics[]` (`DatasetMetric`) only models writes: `bytesWritten`, `entitiesWritten`, `successfullyCommitted`, `dataQualityStatus`, and, since GOBBLIN-2269, `snapshotsCommitted` / `partitionsCommitted`. Data-retention (deletion) jobs have no way to report per-dataset work through this channel. A job that removes aged-out dataset versions can already emit one `datasetTaskSummaries` entry per dataset root, but the deletion counts are dropped when GaaS parses them into `DatasetTaskSummary`.
This PR adds three optional, retention-only counts and threads them through. It follows the same pattern as GOBBLIN-2269 (#4206).
- **`GaaSJobObservabilityEvent.avsc`**: `DatasetMetric` gains three fields, each `["null","long"]` with `default: null`:
- `versionsTotal`: versions considered under the dataset root (deletable + retained);
- `versionsDeleted`: versions deleted, or on a dry run, those that would be;
- `directoriesPruned`: now-empty parent directories pruned after the deletes.
Because they are nullable with null defaults, the schema change is backward compatible, and every existing producer and consumer is unaffected.
- **`DatasetTaskSummary`**: adds the three fields as nullable `Long`, intentionally **not** `@NonNull`. That keeps the 5-arg `@RequiredArgsConstructor` used by `AbstractJobLauncher` and `CommitActivityImpl` intact. The fields are populated by Gson during deserialization of events that carry them. `toDatasetMetric` maps them through, and they stay `null` for write jobs.
The first producer is the ddm-executor retention run (linkedin-multiproduct/ddm-executor#507). It emits one row per dataset root in a single `JobSummaryTimer` event, rather than one event per dataset, which would collide on the job-execution identity. Each row carries `recordsWritten = bytesWritten = 0` plus these counts.
### Tests
- [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
New `DatasetTaskSummaryTest`:
- `testToDatasetMetricMapsRetentionCountsFromJson`: parses the exact JSON a retention run emits, via `GsonUtils.GSON_WITH_DATE_HANDLING` (the path `GaaSJobObservabilityEventProducer` uses), and asserts the counts reach `DatasetMetric`.
- `testToDatasetMetricLeavesRetentionCountsNullForWriteJobs`: the existing 5-arg write-job summary maps the new fields to `null`.
The existing `GaaSJobObservabilityProducerTest` still passes, so write-job `datasetsMetrics` are unchanged.
### Commits
- [x] My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GOBBLIN-2271] Add retention (deletion) counts to DatasetMetric in GaaSJobObservabilityEvent [gobblin]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4209:
URL: https://github.com/apache/gobblin/pull/4209#discussion_r4259472774
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/DatasetTaskSummary.java:
##########
@@ -51,11 +51,17 @@ public class DatasetTaskSummary {
// run can commit more than one snapshot/partition); null = unsupported/unreported.
private String snapshotsCommitted;
private String partitionsCommitted;
+ // Retention (data-deletion) counts, likewise not @NonNull and populated during JSON deserialization of
+ // events that carry them (e.g. DDM retention runs); null = not a deletion job.
+ private Long versionsTotal;
+ private Long versionsDeleted;
+ private Long directoriesPruned;
/**
* Convert a {@link DatasetTaskSummary} to a {@link DatasetMetric}.
*/
public static DatasetMetric toDatasetMetric(DatasetTaskSummary datasetTaskSummary) {
- return new DatasetMetric(datasetTaskSummary.getDatasetUrn(), datasetTaskSummary.getBytesWritten(), datasetTaskSummary.getRecordsWritten(), datasetTaskSummary.isSuccessfullyCommitted(), datasetTaskSummary.getDataQualityStatus(), datasetTaskSummary.getSnapshotsCommitted(), datasetTaskSummary.getPartitionsCommitted());
+ return new DatasetMetric(datasetTaskSummary.getDatasetUrn(), datasetTaskSummary.getBytesWritten(), datasetTaskSummary.getRecordsWritten(), datasetTaskSummary.isSuccessfullyCommitted(), datasetTaskSummary.getDataQualityStatus(), datasetTaskSummary.getSnapshotsCommitted(), datasetTaskSummary.getPartitionsCommitted(),
+ datasetTaskSummary.getVersionsTotal(), datasetTaskSummary.getVersionsDeleted(), datasetTaskSummary.getDirectoriesPruned());
Review Comment:
Using a long positional constructor call is brittle and easy to break as the Avro schema evolves (parameter order errors are hard to spot in review). Prefer constructing `DatasetMetric` via its Avro builder (or setters) so fields are clearly named and adding future fields is less error-prone.
##########
gobblin-runtime/src/test/java/org/apache/gobblin/runtime/DatasetTaskSummaryTest.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.gobblin.runtime;
+
+import java.lang.reflect.Type;
+import java.util.List;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import com.google.gson.reflect.TypeToken;
+
+import org.apache.gobblin.metrics.DatasetMetric;
+import org.apache.gobblin.runtime.util.GsonUtils;
+
+
+public class DatasetTaskSummaryTest {
+
+ private static final Type SUMMARY_LIST = new TypeToken<List<DatasetTaskSummary>>() { }.getType();
+
+ @Test
+ public void testToDatasetMetricMapsRetentionCountsFromJson() {
+ // The shape a DDM retention run emits: nothing written, deletion counts set.
+ String json = "[{\"datasetUrn\":\"hdfs://nn/data/foo\",\"recordsWritten\":0,\"bytesWritten\":0,"
+ + "\"successfullyCommitted\":true,\"versionsTotal\":10,\"versionsDeleted\":4,\"directoriesPruned\":1}]";
+ List<DatasetTaskSummary> summaries = GsonUtils.GSON_WITH_DATE_HANDLING.fromJson(json, SUMMARY_LIST);
Review Comment:
The constant name `SUMMARY_LIST` is a bit ambiguous since it’s actually a `Type` token for a list of summaries. Consider renaming to something clearer like `SUMMARY_LIST_TYPE` (or similar) to reflect what it represents.
##########
gobblin-runtime/src/test/java/org/apache/gobblin/runtime/DatasetTaskSummaryTest.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.gobblin.runtime;
+
+import java.lang.reflect.Type;
+import java.util.List;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import com.google.gson.reflect.TypeToken;
+
+import org.apache.gobblin.metrics.DatasetMetric;
+import org.apache.gobblin.runtime.util.GsonUtils;
+
+
+public class DatasetTaskSummaryTest {
+
+ private static final Type SUMMARY_LIST = new TypeToken<List<DatasetTaskSummary>>() { }.getType();
+
+ @Test
+ public void testToDatasetMetricMapsRetentionCountsFromJson() {
+ // The shape a DDM retention run emits: nothing written, deletion counts set.
+ String json = "[{\"datasetUrn\":\"hdfs://nn/data/foo\",\"recordsWritten\":0,\"bytesWritten\":0,"
+ + "\"successfullyCommitted\":true,\"versionsTotal\":10,\"versionsDeleted\":4,\"directoriesPruned\":1}]";
+ List<DatasetTaskSummary> summaries = GsonUtils.GSON_WITH_DATE_HANDLING.fromJson(json, SUMMARY_LIST);
+
+ DatasetMetric metric = DatasetTaskSummary.toDatasetMetric(summaries.get(0));
+
+ Assert.assertEquals(metric.getDatasetUrn(), "hdfs://nn/data/foo");
+ Assert.assertEquals(metric.getEntitiesWritten(), Long.valueOf(0L));
+ Assert.assertEquals(metric.getVersionsTotal(), Long.valueOf(10L));
+ Assert.assertEquals(metric.getVersionsDeleted(), Long.valueOf(4L));
+ Assert.assertEquals(metric.getDirectoriesPruned(), Long.valueOf(1L));
Review Comment:
This test validates the new retention counters, but it doesn’t assert other key fields present in the JSON (e.g., `bytesWritten` and `successfullyCommitted`). Adding assertions for those fields would better protect against regressions in JSON deserialization or mapping (e.g., fields getting dropped/renamed).
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GOBBLIN-2271] Add retention (deletion) counts to DatasetMetric in GaaSJobObservabilityEvent [gobblin]
Posted by "agam-99 (via GitHub)" <gi...@apache.org>.
agam-99 commented on code in PR #4209:
URL: https://github.com/apache/gobblin/pull/4209#discussion_r4268929386
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/DatasetTaskSummary.java:
##########
@@ -51,11 +51,17 @@ public class DatasetTaskSummary {
// run can commit more than one snapshot/partition); null = unsupported/unreported.
private String snapshotsCommitted;
private String partitionsCommitted;
+ // Retention (data-deletion) counts, likewise not @NonNull and populated during JSON deserialization of
+ // events that carry them (e.g. DDM retention runs); null = not a deletion job.
+ private Long versionsTotal;
+ private Long versionsDeleted;
+ private Long directoriesPruned;
/**
* Convert a {@link DatasetTaskSummary} to a {@link DatasetMetric}.
*/
public static DatasetMetric toDatasetMetric(DatasetTaskSummary datasetTaskSummary) {
- return new DatasetMetric(datasetTaskSummary.getDatasetUrn(), datasetTaskSummary.getBytesWritten(), datasetTaskSummary.getRecordsWritten(), datasetTaskSummary.isSuccessfullyCommitted(), datasetTaskSummary.getDataQualityStatus(), datasetTaskSummary.getSnapshotsCommitted(), datasetTaskSummary.getPartitionsCommitted());
+ return new DatasetMetric(datasetTaskSummary.getDatasetUrn(), datasetTaskSummary.getBytesWritten(), datasetTaskSummary.getRecordsWritten(), datasetTaskSummary.isSuccessfullyCommitted(), datasetTaskSummary.getDataQualityStatus(), datasetTaskSummary.getSnapshotsCommitted(), datasetTaskSummary.getPartitionsCommitted(),
+ datasetTaskSummary.getVersionsTotal(), datasetTaskSummary.getVersionsDeleted(), datasetTaskSummary.getDirectoriesPruned());
Review Comment:
Agreed. `toDatasetMetric` now builds the record with `DatasetMetric.newBuilder()` and named setters for all ten fields, so field order no longer matters and a future schema field is one added `.setX(...)`. Done in d6cd8d97f.
##########
gobblin-runtime/src/test/java/org/apache/gobblin/runtime/DatasetTaskSummaryTest.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.gobblin.runtime;
+
+import java.lang.reflect.Type;
+import java.util.List;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import com.google.gson.reflect.TypeToken;
+
+import org.apache.gobblin.metrics.DatasetMetric;
+import org.apache.gobblin.runtime.util.GsonUtils;
+
+
+public class DatasetTaskSummaryTest {
+
+ private static final Type SUMMARY_LIST = new TypeToken<List<DatasetTaskSummary>>() { }.getType();
+
+ @Test
+ public void testToDatasetMetricMapsRetentionCountsFromJson() {
+ // The shape a DDM retention run emits: nothing written, deletion counts set.
+ String json = "[{\"datasetUrn\":\"hdfs://nn/data/foo\",\"recordsWritten\":0,\"bytesWritten\":0,"
+ + "\"successfullyCommitted\":true,\"versionsTotal\":10,\"versionsDeleted\":4,\"directoriesPruned\":1}]";
+ List<DatasetTaskSummary> summaries = GsonUtils.GSON_WITH_DATE_HANDLING.fromJson(json, SUMMARY_LIST);
Review Comment:
Renamed to `SUMMARY_LIST_TYPE`. Done in d6cd8d97f.
##########
gobblin-runtime/src/test/java/org/apache/gobblin/runtime/DatasetTaskSummaryTest.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.gobblin.runtime;
+
+import java.lang.reflect.Type;
+import java.util.List;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import com.google.gson.reflect.TypeToken;
+
+import org.apache.gobblin.metrics.DatasetMetric;
+import org.apache.gobblin.runtime.util.GsonUtils;
+
+
+public class DatasetTaskSummaryTest {
+
+ private static final Type SUMMARY_LIST = new TypeToken<List<DatasetTaskSummary>>() { }.getType();
+
+ @Test
+ public void testToDatasetMetricMapsRetentionCountsFromJson() {
+ // The shape a DDM retention run emits: nothing written, deletion counts set.
+ String json = "[{\"datasetUrn\":\"hdfs://nn/data/foo\",\"recordsWritten\":0,\"bytesWritten\":0,"
+ + "\"successfullyCommitted\":true,\"versionsTotal\":10,\"versionsDeleted\":4,\"directoriesPruned\":1}]";
+ List<DatasetTaskSummary> summaries = GsonUtils.GSON_WITH_DATE_HANDLING.fromJson(json, SUMMARY_LIST);
+
+ DatasetMetric metric = DatasetTaskSummary.toDatasetMetric(summaries.get(0));
+
+ Assert.assertEquals(metric.getDatasetUrn(), "hdfs://nn/data/foo");
+ Assert.assertEquals(metric.getEntitiesWritten(), Long.valueOf(0L));
+ Assert.assertEquals(metric.getVersionsTotal(), Long.valueOf(10L));
+ Assert.assertEquals(metric.getVersionsDeleted(), Long.valueOf(4L));
+ Assert.assertEquals(metric.getDirectoriesPruned(), Long.valueOf(1L));
Review Comment:
Added assertions for `bytesWritten` and `successfullyCommitted`, so the test now checks the write fields DDM sends alongside the retention counts. Done in d6cd8d97f.
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GOBBLIN-2271] Add retention (deletion) counts to DatasetMetric in GaaSJobObservabilityEvent [gobblin]
Posted by "Blazer-007 (via GitHub)" <gi...@apache.org>.
Blazer-007 merged PR #4209:
URL: https://github.com/apache/gobblin/pull/4209
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org