Skip to content

Conversation

@zjffdu
Copy link
Contributor

@zjffdu zjffdu commented May 7, 2020

What is this PR for?

This PR is to use isolated interpreter for cron note job. Cron job is for production job, so we should use dedicated interpreter for it instead of using shared interpreter with other notes or users. Currently interpreter binding mode is determined by note or users (scoped or isolated), This PR introduce ExecutionContext which include other factors (isCronMode) to determine how we create interpreters.

What type of PR is it?

[Feature]

Todos

  • - Task

What is the Jira issue?

How should this be tested?

  • Unit test is updated, and manually tested

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No


private String user;
private String noteId;
private boolean inCronMode;
Copy link
Contributor

Choose a reason for hiding this comment

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

You should add final, because you have no setter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch,will fix it.

}

private String getInterpreterGroupId(String user, String noteId) {
private static DateTimeFormatter dtf = DateTimeFormatter.ofPattern("yyyy-MM-dd_HH-mm-ss");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should also be an candidate for the final keyword.

@Reamer
Copy link
Contributor

Reamer commented May 7, 2020

LGTM. Can you solve the merge conflict in QuartzSchedulerService.java.

@zjffdu zjffdu force-pushed the ZEPPELIN-4763 branch 3 times, most recently from a5f2ec5 to 39147c3 Compare May 8, 2020 06:16
@asfgit asfgit closed this in cd8a704 May 8, 2020
asfgit pushed a commit that referenced this pull request May 8, 2020
### What is this PR for?

This PR is to use isolated interpreter for cron note job. Cron job is for production job, so we should use dedicated interpreter for it instead of using shared interpreter with other notes or users.  Currently interpreter binding mode is determined by note or users (scoped or isolated), This PR introduce `ExecutionContext` which include other factors (isCronMode) to determine how we create interpreters.

### What type of PR is it?
[Feature]

### Todos
* [ ] - Task

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-4763

### How should this be tested?
* Unit test is updated, and manually tested

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Jeff Zhang <[email protected]>

Closes #3765 from zjffdu/ZEPPELIN-4763 and squashes the following commits:

39147c3 [Jeff Zhang] address comment
7bc690d [Jeff Zhang] [ZEPPELIN-4763]. Use isolated interpreter for cron note job

(cherry picked from commit cd8a704)
Signed-off-by: Jeff Zhang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants