You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@gobblin.apache.org by GitBox <gi...@apache.org> on 2020/03/31 20:30:11 UTC

[GitHub] [incubator-gobblin] arjun4084346 opened a new pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

arjun4084346 opened a new pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945

Dear Gobblin maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
@zxcware please review

### JIRA
- [x] My PR addresses the following [Gobblin JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
- https://issues.apache.org/jira/browse/GOBBLIN-1105

### Description
- [x] Here are some details about my PR, including screenshots (if applicable):
make MysqlJobStatusStateStore implements DatasetStateStore so that we can use existing cleaners/dataset finders/version finders to clean job status

### Tests
- [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
test cases already exist

### 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

[GitHub] [incubator-gobblin] asfgit closed pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

asfgit closed pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945
 
 
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

sv2000 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#discussion_r402067225
 
 

 ##########
 File path: gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlStateStore.java
 ##########
 @@ -73,7 +74,7 @@
  *
  * @param <T> state object type
  **/
-public class MysqlStateStore<T extends State> implements StateStore<T> {
+public class MysqlStateStore<T extends State> implements StateStore<T>, DatasetStateStore<T> {
 
 Review comment:
   Instead of changing MySqlStateStore (and indeed, other state store implementations), can we have MySqlJobStatusStateStore implement DatasetStateStore interface? Won't that change be more localized?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#issuecomment-606889224
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=h1) Report
   > Merging [#2945](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d54e66b857a34e871fd7c9ab28e37c9c9f061432&el=desc) will **decrease** coverage by `0.04%`.
   > The diff coverage is `1.63%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/graphs/tree.svg?width=650&height=150&src=pr&token=4MgURJ0bGc)](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2945      +/-   ##
   ============================================
   - Coverage     45.58%   45.54%   -0.05%     
   + Complexity     9157     9155       -2     
   ============================================
     Files          1936     1938       +2     
     Lines         73286    73323      +37     
     Branches       8088     8091       +3     
   ============================================
   - Hits          33409    33393      -16     
   - Misses        36782    36835      +53     
     Partials       3095     3095              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...a/org/apache/gobblin/cluster/HelixJobsMapping.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhKb2JzTWFwcGluZy5qYXZh) | `89.58% <0.00%> (+6.89%)` | `13.00 <0.00> (ø)` | |
   | [...rg/apache/gobblin/metastore/DatasetStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0RhdGFzZXRTdGF0ZVN0b3JlLmphdmE=) | `4.34% <ø> (+4.34%)` | `1.00 <0.00> (+1.00)` | |
   | [.../apache/gobblin/metastore/DatasetStoreDataset.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0RhdGFzZXRTdG9yZURhdGFzZXQuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...metastore/FileContextBasedFsStateStoreFactory.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0ZpbGVDb250ZXh0QmFzZWRGc1N0YXRlU3RvcmVGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ava/org/apache/gobblin/metastore/FsStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0ZzU3RhdGVTdG9yZS5qYXZh) | `32.91% <0.00%> (-3.14%)` | `14.00 <0.00> (ø)` | |
   | [...he/gobblin/metastore/MysqlJobStatusStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZS5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...etastore/MysqlJobStatusStateStoreEntryManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZUVudHJ5TWFuYWdlci5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...lin/metastore/MysqlJobStatusStateStoreFactory.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZUZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../org/apache/gobblin/metastore/MysqlStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsU3RhdGVTdG9yZS5qYXZh) | `8.19% <ø> (ø)` | `2.00 <0.00> (ø)` | |
   | [...gobblin/metastore/MysqlStateStoreEntryManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsU3RhdGVTdG9yZUVudHJ5TWFuYWdlci5qYXZh) | `0.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | ... and [16 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=footer). Last update [d54e66b...9689853](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

sv2000 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#discussion_r401265573
 
 

 ##########
 File path: gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlJobStatusStateStore.java
 ##########
 @@ -33,7 +33,7 @@
  *
  * @param <T> state object type
  **/
-public class MysqlJobStatusStateStore<T extends State> extends MysqlStateStore {
+public class MysqlJobStatusStateStore<T extends State> extends MysqlStateStore implements DatasetStateStore {
 
 Review comment:
   Can't MySqlJobStatusStateStore extend MysqlDatasetStateStore directly?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

sv2000 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#discussion_r403242940
 
 

 ##########
 File path: gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlJobStatusStateStore.java
 ##########
 @@ -50,12 +61,63 @@ public MysqlJobStatusStateStore(DataSource dataSource, String stateStoreTableNam
 
   /**
    * Returns all the job statuses for a flow group, flow name, flow execution id
-   * @param storeName
-   * @param flowExecutionId
-   * @return
-   * @throws IOException
+   * @param storeName store name in the state store
+   * @param flowExecutionId Flow Execution Id
+   * @return list of states
+   * @throws IOException in case of failures
    */
   public List<T> getAll(String storeName, long flowExecutionId) throws IOException {
     return getAll(storeName, flowExecutionId + "%", true);
   }
+
+  @Override
+  public List<DatasetStateStoreEntryManager<T>> getMetadataForTables(StateStorePredicate predicate)
 
 Review comment:
   Ok. Resolving this comment, since this class needs metrics emission from other methods as well. Makes sense to create a separate PR for that.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] arjun4084346 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

arjun4084346 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#discussion_r403239504
 
 

 ##########
 File path: gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlJobStatusStateStore.java
 ##########
 @@ -50,12 +61,63 @@ public MysqlJobStatusStateStore(DataSource dataSource, String stateStoreTableNam
 
   /**
    * Returns all the job statuses for a flow group, flow name, flow execution id
-   * @param storeName
-   * @param flowExecutionId
-   * @return
-   * @throws IOException
+   * @param storeName store name in the state store
+   * @param flowExecutionId Flow Execution Id
+   * @return list of states
+   * @throws IOException in case of failures
    */
   public List<T> getAll(String storeName, long flowExecutionId) throws IOException {
     return getAll(storeName, flowExecutionId + "%", true);
   }
+
+  @Override
+  public List<DatasetStateStoreEntryManager<T>> getMetadataForTables(StateStorePredicate predicate)
 
 Review comment:
   We can add metrics in other PR.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] arjun4084346 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

arjun4084346 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#discussion_r401274324
 
 

 ##########
 File path: gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlStateStore.java
 ##########
 @@ -73,7 +74,7 @@
  *
  * @param <T> state object type
  **/
-public class MysqlStateStore<T extends State> implements StateStore<T> {
+public class MysqlStateStore<T extends State> implements StateStore<T>, DatasetStateStore<T> {
 
 Review comment:
   Oh, that is not done intentionally, after making this class implement DatasetStateStore, I forgot to to make its child class to not implement DatasetStateStore

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] arjun4084346 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

arjun4084346 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#discussion_r401271572
 
 

 ##########
 File path: gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlJobStatusStateStore.java
 ##########
 @@ -33,7 +33,7 @@
  *
  * @param <T> state object type
  **/
-public class MysqlJobStatusStateStore<T extends State> extends MysqlStateStore {
+public class MysqlJobStatusStateStore<T extends State> extends MysqlStateStore implements DatasetStateStore {
 
 Review comment:
   Not easily.
   MysqlDatasetStateStore implements DatasetStateStore<JobState.DatasetState>
   we want MysqlDatasetStateStore to implement DatasetStateStore<State>
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

sv2000 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#discussion_r401264580
 
 

 ##########
 File path: gobblin-metastore/src/main/java/org/apache/gobblin/metastore/DatasetStoreDataset.java
 ##########
 @@ -36,7 +38,8 @@
 
   @Override
   public String datasetURN() {
-    return this.key.getStoreName() + ":::" + this.key.getSanitizedDatasetUrn();
+    return this.key.getStoreName() +
+        (StringUtils.isEmpty(this.key.getSanitizedDatasetUrn()) ? "" : ":::" + this.key.getSanitizedDatasetUrn());
 
 Review comment:
   Nit: Guava's Strings.isNullOrEmpty() is more readable. I did not realize until I saw the Javadoc for StringUtils.isEmpty that it returns true if the string is null.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] zxcware commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

zxcware commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#discussion_r402643365
 
 

 ##########
 File path: gobblin-metastore/src/main/java/org/apache/gobblin/metastore/FsStateStore.java
 ##########
 @@ -378,4 +387,48 @@ public void delete(String storeName) throws IOException {
       this.fs.delete(storePath, true);
     }
   }
+
+  @Override
+  public Map<String, T> getLatestDatasetStatesByUrns(String jobName) throws IOException {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public T getLatestDatasetState(String storeName, String datasetUrn) throws IOException {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public void persistDatasetState(String datasetUrn, T datasetState) throws IOException {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public void persistDatasetURNs(String storeName, Collection<String> datasetUrns) throws IOException {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public List<? extends StateStoreEntryManager> getMetadataForTables(StateStorePredicate predicate) {
 
 Review comment:
    Should it be `DatasetStateStoreEntryManager` to correct serve its role as dataset state store?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

sv2000 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#discussion_r403233301
 
 

 ##########
 File path: gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlJobStatusStateStore.java
 ##########
 @@ -50,12 +61,63 @@ public MysqlJobStatusStateStore(DataSource dataSource, String stateStoreTableNam
 
   /**
    * Returns all the job statuses for a flow group, flow name, flow execution id
-   * @param storeName
-   * @param flowExecutionId
-   * @return
-   * @throws IOException
+   * @param storeName store name in the state store
+   * @param flowExecutionId Flow Execution Id
+   * @return list of states
+   * @throws IOException in case of failures
    */
   public List<T> getAll(String storeName, long flowExecutionId) throws IOException {
     return getAll(storeName, flowExecutionId + "%", true);
   }
+
+  @Override
+  public List<DatasetStateStoreEntryManager<T>> getMetadataForTables(StateStorePredicate predicate)
 
 Review comment:
   Can we emit a metric for the latency of the MySQL query?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

sv2000 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#discussion_r401267437
 
 

 ##########
 File path: gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlStateStore.java
 ##########
 @@ -73,7 +74,7 @@
  *
  * @param <T> state object type
  **/
-public class MysqlStateStore<T extends State> implements StateStore<T> {
+public class MysqlStateStore<T extends State> implements StateStore<T>, DatasetStateStore<T> {
 
 Review comment:
   This is looking strange. The child class MySqlDatasetStateStore extends MySqlStateStore and implements DatasetStateStore. And the parent class is implementing the interface. See my comment earlier about considering the possibility of MysqlJobStatusStore extending MySqlDatasetStateStore.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#issuecomment-606889224
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=h1) Report
   > Merging [#2945](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d54e66b857a34e871fd7c9ab28e37c9c9f061432&el=desc) will **decrease** coverage by `0.02%`.
   > The diff coverage is `1.63%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/graphs/tree.svg?width=650&height=150&src=pr&token=4MgURJ0bGc)](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2945      +/-   ##
   ============================================
   - Coverage     45.58%   45.56%   -0.03%     
     Complexity     9157     9157              
   ============================================
     Files          1936     1938       +2     
     Lines         73286    73323      +37     
     Branches       8088     8091       +3     
   ============================================
   + Hits          33409    33410       +1     
   - Misses        36782    36818      +36     
     Partials       3095     3095              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...a/org/apache/gobblin/cluster/HelixJobsMapping.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhKb2JzTWFwcGluZy5qYXZh) | `89.58% <0.00%> (+6.89%)` | `13.00 <0.00> (ø)` | |
   | [...rg/apache/gobblin/metastore/DatasetStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0RhdGFzZXRTdGF0ZVN0b3JlLmphdmE=) | `4.34% <ø> (+4.34%)` | `1.00 <0.00> (+1.00)` | |
   | [.../apache/gobblin/metastore/DatasetStoreDataset.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0RhdGFzZXRTdG9yZURhdGFzZXQuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...metastore/FileContextBasedFsStateStoreFactory.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0ZpbGVDb250ZXh0QmFzZWRGc1N0YXRlU3RvcmVGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ava/org/apache/gobblin/metastore/FsStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0ZzU3RhdGVTdG9yZS5qYXZh) | `32.91% <0.00%> (-3.14%)` | `14.00 <0.00> (ø)` | |
   | [...he/gobblin/metastore/MysqlJobStatusStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZS5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...etastore/MysqlJobStatusStateStoreEntryManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZUVudHJ5TWFuYWdlci5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...lin/metastore/MysqlJobStatusStateStoreFactory.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZUZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../org/apache/gobblin/metastore/MysqlStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsU3RhdGVTdG9yZS5qYXZh) | `8.19% <ø> (ø)` | `2.00 <0.00> (ø)` | |
   | [...ache/gobblin/metastore/MysqlStateStoreFactory.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsU3RhdGVTdG9yZUZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | ... and [14 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=footer). Last update [d54e66b...30585d0](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#issuecomment-606889224
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=h1) Report
   > Merging [#2945](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d54e66b857a34e871fd7c9ab28e37c9c9f061432&el=desc) will **decrease** coverage by `0.01%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/graphs/tree.svg?width=650&height=150&src=pr&token=4MgURJ0bGc)](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2945      +/-   ##
   ============================================
   - Coverage     45.58%   45.56%   -0.02%     
   - Complexity     9157     9160       +3     
   ============================================
     Files          1936     1937       +1     
     Lines         73286    73322      +36     
     Branches       8088     8092       +4     
   ============================================
   + Hits          33409    33412       +3     
   - Misses        36782    36814      +32     
   - Partials       3095     3096       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...a/org/apache/gobblin/cluster/HelixJobsMapping.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhKb2JzTWFwcGluZy5qYXZh) | `89.58% <0.00%> (+6.89%)` | `13.00 <0.00> (ø)` | |
   | [.../apache/gobblin/metastore/DatasetStoreDataset.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0RhdGFzZXRTdG9yZURhdGFzZXQuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...he/gobblin/metastore/MysqlJobStatusStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZS5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...etastore/MysqlJobStatusStateStoreEntryManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZUVudHJ5TWFuYWdlci5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...lin/metastore/MysqlJobStatusStateStoreFactory.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZUZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../org/apache/gobblin/metastore/MysqlStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsU3RhdGVTdG9yZS5qYXZh) | `8.19% <ø> (ø)` | `2.00 <0.00> (ø)` | |
   | [...ache/gobblin/metastore/MysqlStateStoreFactory.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsU3RhdGVTdG9yZUZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...astore/metadata/DatasetStateStoreEntryManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL21ldGFkYXRhL0RhdGFzZXRTdGF0ZVN0b3JlRW50cnlNYW5hZ2VyLmphdmE=) | `0.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../apache/gobblin/metastore/ZkStateStoreFactory.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4taGVsaXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL1prU3RhdGVTdG9yZUZhY3RvcnkuamF2YQ==) | `71.42% <ø> (ø)` | `2.00 <0.00> (ø)` | |
   | [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `85.71% <0.00%> (-7.15%)` | `3.00% <0.00%> (ø%)` | |
   | ... and [7 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=footer). Last update [d54e66b...f325b74](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] zxcware commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

zxcware commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#discussion_r403204430
 
 

 ##########
 File path: gobblin-metastore/src/main/java/org/apache/gobblin/metastore/DatasetStateStore.java
 ##########
 @@ -56,7 +56,7 @@
   public void persistDatasetURNs(String storeName, Collection<String> datasetUrns) throws IOException;
 
   @Override
-  default List<? extends DatasetStateStoreEntryManager> getMetadataForTables(StateStorePredicate predicate)
+  default List<? extends StateStoreEntryManager> getMetadataForTables(StateStorePredicate predicate)
 
 Review comment:
   For `DatasetStateStore`, should use `DatasetStateStoreEntryManager`, right?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] arjun4084346 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

arjun4084346 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#discussion_r401271572
 
 

 ##########
 File path: gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlJobStatusStateStore.java
 ##########
 @@ -33,7 +33,7 @@
  *
  * @param <T> state object type
  **/
-public class MysqlJobStatusStateStore<T extends State> extends MysqlStateStore {
+public class MysqlJobStatusStateStore<T extends State> extends MysqlStateStore implements DatasetStateStore {
 
 Review comment:
   Not easily.
   MysqlDatasetStateStore implements DatasetStateStore<JobState.DatasetState>
   we want MysqlDatasetStateStore to implement DatasetStateStore<State>
   .

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#issuecomment-606889224
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=h1) Report
   > Merging [#2945](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d54e66b857a34e871fd7c9ab28e37c9c9f061432&el=desc) will **decrease** coverage by `0.04%`.
   > The diff coverage is `1.63%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/graphs/tree.svg?width=650&height=150&src=pr&token=4MgURJ0bGc)](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2945      +/-   ##
   ============================================
   - Coverage     45.58%   45.54%   -0.05%     
   + Complexity     9157     9155       -2     
   ============================================
     Files          1936     1938       +2     
     Lines         73286    73323      +37     
     Branches       8088     8091       +3     
   ============================================
   - Hits          33409    33393      -16     
   - Misses        36782    36835      +53     
     Partials       3095     3095              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...a/org/apache/gobblin/cluster/HelixJobsMapping.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhKb2JzTWFwcGluZy5qYXZh) | `89.58% <0.00%> (+6.89%)` | `13.00 <0.00> (ø)` | |
   | [...rg/apache/gobblin/metastore/DatasetStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0RhdGFzZXRTdGF0ZVN0b3JlLmphdmE=) | `4.34% <ø> (+4.34%)` | `1.00 <0.00> (+1.00)` | |
   | [.../apache/gobblin/metastore/DatasetStoreDataset.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0RhdGFzZXRTdG9yZURhdGFzZXQuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...metastore/FileContextBasedFsStateStoreFactory.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0ZpbGVDb250ZXh0QmFzZWRGc1N0YXRlU3RvcmVGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ava/org/apache/gobblin/metastore/FsStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0ZzU3RhdGVTdG9yZS5qYXZh) | `32.91% <0.00%> (-3.14%)` | `14.00 <0.00> (ø)` | |
   | [...he/gobblin/metastore/MysqlJobStatusStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZS5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...etastore/MysqlJobStatusStateStoreEntryManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZUVudHJ5TWFuYWdlci5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...lin/metastore/MysqlJobStatusStateStoreFactory.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZUZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../org/apache/gobblin/metastore/MysqlStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsU3RhdGVTdG9yZS5qYXZh) | `8.19% <ø> (ø)` | `2.00 <0.00> (ø)` | |
   | [...gobblin/metastore/MysqlStateStoreEntryManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsU3RhdGVTdG9yZUVudHJ5TWFuYWdlci5qYXZh) | `0.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | ... and [16 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=footer). Last update [d54e66b...9689853](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-io commented on issue #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

codecov-io commented on issue #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#issuecomment-606889224
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=h1) Report
   > Merging [#2945](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/7b9c7cbbff76c6963576846b81bfdae8ced4ba22&el=desc) will **decrease** coverage by `0.96%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/graphs/tree.svg?width=650&height=150&src=pr&token=4MgURJ0bGc)](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2945      +/-   ##
   ============================================
   - Coverage     45.58%   44.62%   -0.97%     
   + Complexity     9160     8985     -175     
   ============================================
     Files          1936     1936              
     Lines         73292    73302      +10     
     Branches       8088     8089       +1     
   ============================================
   - Hits          33413    32713     -700     
   - Misses        36784    37538     +754     
   + Partials       3095     3051      -44     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...rg/apache/gobblin/metastore/DatasetStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0RhdGFzZXRTdGF0ZVN0b3JlLmphdmE=) | `4.34% <ø> (+4.34%)` | `1.00 <0.00> (+1.00)` | |
   | [.../apache/gobblin/metastore/DatasetStoreDataset.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL0RhdGFzZXRTdG9yZURhdGFzZXQuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...he/gobblin/metastore/MysqlJobStatusStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZS5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...lin/metastore/MysqlJobStatusStateStoreFactory.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsSm9iU3RhdHVzU3RhdGVTdG9yZUZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../org/apache/gobblin/metastore/MysqlStateStore.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsU3RhdGVTdG9yZS5qYXZh) | `8.06% <0.00%> (-0.14%)` | `2.00 <0.00> (ø)` | |
   | [...gobblin/metastore/MysqlStateStoreEntryManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRhc3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0YXN0b3JlL015c3FsU3RhdGVTdG9yZUVudHJ5TWFuYWdlci5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...gobblin/runtime/mapreduce/GobblinOutputFormat.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbWFwcmVkdWNlL0dvYmJsaW5PdXRwdXRGb3JtYXQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-2.00%)` | |
   | [...askStateCollectorServiceHiveRegHandlerFactory.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvVGFza1N0YXRlQ29sbGVjdG9yU2VydmljZUhpdmVSZWdIYW5kbGVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-2.00%)` | |
   | [...re/filesystem/FsDatasetStateStoreEntryManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbWV0YXN0b3JlL2ZpbGVzeXN0ZW0vRnNEYXRhc2V0U3RhdGVTdG9yZUVudHJ5TWFuYWdlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-3.00%)` | |
   | [...in/runtime/mapreduce/CustomizedProgresserBase.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbWFwcmVkdWNlL0N1c3RvbWl6ZWRQcm9ncmVzc2VyQmFzZS5qYXZh) | `0.00% <0.00%> (-83.34%)` | `0.00% <0.00%> (-1.00%)` | |
   | ... and [49 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2945/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=footer). Last update [7b9c7cb...62c0251](https://codecov.io/gh/apache/incubator-gobblin/pull/2945?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] arjun4084346 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore

Posted by GitBox <gi...@apache.org>.

arjun4084346 commented on a change in pull request #2945: [GOBBLIN-1105] some refactoring and make MysqlJobStatusStateStore implements DatasetStateStore
URL: https://github.com/apache/incubator-gobblin/pull/2945#discussion_r403237650
 
 

 ##########
 File path: gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlJobStatusStateStore.java
 ##########
 @@ -50,12 +61,63 @@ public MysqlJobStatusStateStore(DataSource dataSource, String stateStoreTableNam
 
   /**
    * Returns all the job statuses for a flow group, flow name, flow execution id
-   * @param storeName
-   * @param flowExecutionId
-   * @return
-   * @throws IOException
+   * @param storeName store name in the state store
+   * @param flowExecutionId Flow Execution Id
+   * @return list of states
+   * @throws IOException in case of failures
    */
   public List<T> getAll(String storeName, long flowExecutionId) throws IOException {
     return getAll(storeName, flowExecutionId + "%", true);
   }
+
+  @Override
+  public List<DatasetStateStoreEntryManager<T>> getMetadataForTables(StateStorePredicate predicate)
 
 Review comment:
   This PR is only addressing the refactoring to support cleanup of job statuses

----------------------------------------------------------------
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


With regards,
Apache Git Services