You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gobblin.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2020/05/01 18:26:00 UTC
[jira] [Work logged] (GOBBLIN-1133) Add
CompactionSuiteBaseWithConfigurableCompleteAction to make complete action
configurable
[ https://issues.apache.org/jira/browse/GOBBLIN-1133?focusedWorklogId=429598&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-429598 ]
ASF GitHub Bot logged work on GOBBLIN-1133:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 01/May/20 18:25
Start Date: 01/May/20 18:25
Worklog Time Spent: 10m
Work Description: autumnust commented on a change in pull request #2973:
URL: https://github.com/apache/incubator-gobblin/pull/2973#discussion_r418663974
##########
File path: gobblin-compaction/src/main/java/org/apache/gobblin/compaction/suite/CompactionSuiteBaseWithConfigurableCompleteAction.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.compaction.suite;
+
+import com.google.common.base.Preconditions;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.gobblin.compaction.action.CompactionCompleteAction;
+import org.apache.gobblin.configuration.State;
+import org.apache.gobblin.dataset.FileSystemDataset;
+import org.apache.gobblin.util.reflection.GobblinConstructorUtils;
+
+
+/**
+ * Compaction suit with configurable complete actions
Review comment:
Typo in the comment. suit -> suite
##########
File path: gobblin-compaction/src/main/java/org/apache/gobblin/compaction/suite/CompactionSuiteBaseWithConfigurableCompleteAction.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.compaction.suite;
+
+import com.google.common.base.Preconditions;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.gobblin.compaction.action.CompactionCompleteAction;
+import org.apache.gobblin.configuration.State;
+import org.apache.gobblin.dataset.FileSystemDataset;
+import org.apache.gobblin.util.reflection.GobblinConstructorUtils;
+
+
+/**
+ * Compaction suit with configurable complete actions
+ */
+public class CompactionSuiteBaseWithConfigurableCompleteAction extends CompactionSuiteBase{
+
+ private final static String COMPACTION_COMPLETE_ACTIONS = "compaction.complete.actions";
+ /**
+ * Constructor
+ */
+ public CompactionSuiteBaseWithConfigurableCompleteAction(State state) {
+ super(state);
+ }
+
+ /**
+ * Some post actions are required after compaction job (map-reduce) is finished.
+ *
+ * @return A list of {@link CompactionCompleteAction}s which needs to be executed after
+ * map-reduce is done.
+ */
+ @Override
+ public List<CompactionCompleteAction<FileSystemDataset>> getCompactionCompleteActions() throws IOException {
+ Preconditions.checkArgument(state.contains(COMPACTION_COMPLETE_ACTIONS));
+ ArrayList<CompactionCompleteAction<FileSystemDataset>> array = new ArrayList<>();
Review comment:
I might be nit-pick here but let's name it as `arrayList` since it is more on `list` side ?
##########
File path: gobblin-compaction/src/main/java/org/apache/gobblin/compaction/suite/CompactionSuiteBase.java
##########
@@ -115,7 +115,7 @@ public FileSystemDataset load (final State state) {
* @return A list of {@link CompactionCompleteAction}s which needs to be executed after
* map-reduce is done.
*/
- public List<CompactionCompleteAction<FileSystemDataset>> getCompactionCompleteActions() {
+ public List<CompactionCompleteAction<FileSystemDataset>> getCompactionCompleteActions() throws IOException {
ArrayList<CompactionCompleteAction<FileSystemDataset>> array = new ArrayList<>();
Review comment:
This is not your code but I found it really weird to call this `array`. Can you fix it as part of your PR ? just renaming to arrayList for example is good enough.
##########
File path: gobblin-compaction/src/main/java/org/apache/gobblin/compaction/mapreduce/CompactionJobConfigurator.java
##########
@@ -90,6 +92,11 @@
@Getter
protected Collection<Path> mapReduceInputPaths = null;
@Getter
+ protected Collection<Path> oldFiles = null;
Review comment:
Two things here:
1. Can you add some comments on why these things are needed.
2. Is that possible a dataset have tons of files ? are there concerns on using in-memory data structure here
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 429598)
Time Spent: 0.5h (was: 20m)
> Add CompactionSuiteBaseWithConfigurableCompleteAction to make complete action configurable
> ------------------------------------------------------------------------------------------
>
> Key: GOBBLIN-1133
> URL: https://issues.apache.org/jira/browse/GOBBLIN-1133
> Project: Apache Gobblin
> Issue Type: Task
> Reporter: Zihan Li
> Priority: Major
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> # Add CompactionSuiteBaseWithConfigurableCompleteAction to make complete action configurable
> # Include dstNewFiles and oldFiles in CompactionJobConfigurator
--
This message was sent by Atlassian Jira
(v8.3.4#803005)