Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GOBBLIN-267] Changed workunit creation policy to compare update time with maxLookBackDays #2119

Closed
wants to merge 1 commit into from

Conversation

aditya1105
Copy link
Contributor

@aditya1105 aditya1105 commented Sep 26, 2017

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

Description

  • Here are some details about my PR, including screenshots (if applicable):
    HiveSource#shouldCreateWorkunit() now compares update time with maxLookBackDays

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:
    Very trivial change/fix

Commits

  • 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":
    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"

@aditya1105
Copy link
Contributor Author

@abti, can you please review this pr

* <code>createTime</code> is not used. It exists for backward compatibility
*/
protected boolean shouldCreateWorkunit(long createTime, long updateTime, LongWatermark lowWatermark) {
if (new DateTime(updateTime).isBefore(this.maxLookBackTime)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is updateTime here the same as the partition value (i.e. the time corresponding to the partition value YYYY-MM-DD-HH)? If this is the last time the partition was updated, then I think you might still have a problem. For example, imagine 100 days of partitions are registered today and maxLookBackTime is 5 days ago. The last time all 100 partitions were updated would be > maxLookBackTime and would thus still have workunits created for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update time is controllable via policies.Following are the policies we have:

  1. DatePatternUpdateProvider
  2. HdfsBasedUpdateProvider
  3. HiveMetastoreBasedUpdateProvider

Hence using the 1 and 2 as update policies will make sure workunits won't be created in the scenario mentioned above.
If 3 is used, then both create time and update time should be taken by hive partition property and hence workunits must be created.

Copy link
Contributor

@htran1 htran1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1.

@asfgit asfgit closed this in d195013 Sep 27, 2017
zxliucmu pushed a commit to zxliucmu/incubator-gobblin that referenced this pull request Nov 16, 2017
… with maxLookBackDays

Closes apache#2119 from aditya1105/avro-to-orc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants