Skip to content

[Spark load][Fe 5/6] Fe submit spark etl job - #3716

Merged
morningman merged 1 commit into
apache:masterfrom
wyb:spark_load_fe_submit_etl_job
Jun 19, 2020
Merged

[Spark load][Fe 5/6] Fe submit spark etl job#3716
morningman merged 1 commit into
apache:masterfrom
wyb:spark_load_fe_submit_etl_job

Conversation

@wyb

@wybwyb commented May 28, 2020

Copy link
Copy Markdown
Contributor

After user creates a spark load job which status is PENDING, Fe will schedule and submit the spark etl job.

  1. Begin transaction
  2. Create a SparkLoadPendingTask for submitting etl job
    2.1 Create etl job configuration according to Spark load interface #3010 (comment)
    2.2 Upload the configuration file and job jar to HDFS with broker
    2.3 Submit etl job to spark cluster
    2.4 Wait for etl job submission result
  3. Update job state to ETL and log job update info if etl job is submitted successfully

#3433

@wybwyb mentioned this pull request May 28, 2020
Closed
@imayimay added area/load Issues or PRs related to all kinds of load kind/feature Categorizes issue or PR as related to a new feature. labels May 29, 2020
@imayimay self-assigned this May 29, 2020
imay
imay requested changes May 29, 2020
View reviewed changes
Comment threadfe/src/main/java/org/apache/doris/common/util/BrokerUtil.java Outdated
Comment threadfe/src/main/java/org/apache/doris/common/util/BrokerUtil.java Outdated
Comment threadfe/src/main/java/org/apache/doris/common/util/BrokerUtil.java Outdated
morningman
morningman requested changes May 30, 2020
View reviewed changes
Comment threadfe/src/main/java/org/apache/doris/catalog/OlapTable.java
Comment threadfe/src/main/java/org/apache/doris/common/Pair.java
public class Pair<F, S> {
public static PairComparator<Pair<?, Comparable>> PAIR_VALUE_COMPARATOR = new PairComparator<>();

@SerializedName(value = "first")

@morningmanmorningmanMay 30, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'am not sure this is ok, cause there is no guarantee that the F and S object can also be serialized by GSON

@wybwybJun 1, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Users guarantee this when use Pair class？like Map and List.

@morningmanmorningmanJun 10, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I checked, this is not work

@wybwybJun 18, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I add a comment.
When using Pair for persistence, users need to guarantee that F and S can be serialized through Gson

Comment threadfe/src/main/java/org/apache/doris/common/util/BrokerUtil.java Outdated
Comment threadfe/src/main/java/org/apache/doris/load/loadv2/SparkEtlJobHandler.java
private static final String CONFIG_FILE_NAME = "jobconfig.json";
private static final String APP_RESOURCE_LOCAL_PATH = PaloFe.DORIS_HOME_DIR + "/lib/" + APP_RESOURCE_NAME;
private static final String JOB_CONFIG_DIR = "configs";
private static final String MAIN_CLASS = "org.apache.doris.load.loadv2.etl.SparkEtlJob";

@morningmanmorningmanMay 30, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about get it from SparkEtlJob.class.getXXX()?

@wybwybJun 1, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

ok, I comment and will replace with it when SparkEtlJob class is merged.

Comment threadfe/src/main/java/org/apache/doris/load/loadv2/SparkEtlJobHandler.java
throw new LoadException(errMsg + "spark app state: " + state.toString());
}
if (retry >= GET_APPID_MAX_RETRY_TIMES) {
throw new LoadException(errMsg + "wait too much time for getting appid. spark app state: "

@morningmanmorningmanMay 30, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
thrownewLoadException(errMsg + "wait too much time for getting appid. spark app state: "
thrownewLoadException(errMsg + "wait too much time for getting appi d. spark app state: "

@wybwybJun 1, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

errMsg already have a space at the end.

@wybwyb changed the title [Spark load] Fe submit spark etl job[Spark load] [Fe 4/5] Fe submit spark etl jobMay 30, 2020
@wybwyb changed the title [Spark load] [Fe 4/5] Fe submit spark etl job[Spark load][Fe 4/5] Fe submit spark etl jobMay 30, 2020
@wybwyb changed the title [Spark load][Fe 4/5] Fe submit spark etl job[Spark load][Fe 5/6] Fe submit spark etl jobJun 10, 2020
@wyb
wybforce-pushed the spark_load_fe_submit_etl_job branch 2 times, most recently from 6d0ef4e to 20df6f2CompareJune 13, 2020 13:06
morningman
morningman reviewed Jun 17, 2020
View reviewed changes
Comment threadfe/src/main/java/org/apache/doris/common/Pair.java
public class Pair<F, S> {
public static PairComparator<Pair<?, Comparable>> PAIR_VALUE_COMPARATOR = new PairComparator<>();

@SerializedName(value = "first")

@morningmanmorningmanJun 10, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I checked, this is not work

Comment threadfe/src/main/java/org/apache/doris/common/util/BrokerUtil.java
+ ", msg=" + tReadResponse.getOpStatus().getMessage());
}
failed = false;
return tReadResponse.getData();

@morningmanmorningmanJun 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

broker's pread() method does not guarantee to read the specified length of data currently.
But #3881 is trying to solve this problem. Just for remind.

@wybwybJun 18, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

ok

Comment threadfe/src/main/java/org/apache/doris/load/loadv2/SparkLoadJob.java
morningman
morningman previously approved these changes Jun 18, 2020
View reviewed changes

@morningmanmorningman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

imay
imay previously approved these changes Jun 18, 2020
View reviewed changes
@wyb
wyb dismissed stale reviews from imay and morningman via ee27b34June 18, 2020 08:51
imay
imay previously approved these changes Jun 18, 2020
View reviewed changes

@imayimay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@wyb
wybforce-pushed the spark_load_fe_submit_etl_job branch from ee27b34 to efcc796CompareJune 19, 2020 02:44
morningman
morningman approved these changes Jun 19, 2020
View reviewed changes

@morningmanmorningman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@morningmanmorningman added the approved Indicates a PR has been approved by one committer. label Jun 19, 2020
@morningman
morningman merged commit 532d15d into apache:masterJun 19, 2020
@EmmyMiao87EmmyMiao87 mentioned this pull request Sep 1, 2020
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@morningmanmorningmanmorningman approved these changes
@imayimayimay left review comments

Assignees

@imayimay

Labels

approvedIndicates a PR has been approved by one committer.area/loadIssues or PRs related to all kinds of loadkind/featureCategorizes issue or PR as related to a new feature.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@wyb@imay@morningman