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

FDPIN-3560: Job and org analysis support to Dr Elephant #4

Open
wants to merge 28 commits into
base: working
Choose a base branch
from

Conversation

shubhamjainflipkart
Copy link

No description provided.

@@ -0,0 +1,69 @@
package com.linkedin.drelephant.analysis;

public class HeuristicDetails {
Copy link

Choose a reason for hiding this comment

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

Why are setters needed. Lets make it immutable if it can be done.

Copy link

Choose a reason for hiding this comment

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

Also lets use lombok for the boiler plate?

@@ -73,6 +75,8 @@ private void loadInfo(String appId, Properties properties) {

workflowDepth = 0; // TODO: Add sub-workflow support
jobName = properties.getProperty(AZKABAN_JOB_NAME);
organization = properties.getProperty("job.billing.org");
Copy link

Choose a reason for hiding this comment

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

Please take these out in constants section.

Copy link

Choose a reason for hiding this comment

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

Also does dr elephant have an abstract scheduler? If yes then org, namespace interpretation and logic can probably go there. Also I dont see a need to override them in the concrete impls.

*
* @return the org name
*/
public String getOrganization();
Copy link

Choose a reason for hiding this comment

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

Can this be changed to an abstract class with final implementations for og and sub org so that its available for all schedulers.


import com.google.gson.*;
import static java.lang.StrictMath.max;
import static java.lang.StrictMath.min;


public class Application extends Controller {
Copy link

Choose a reason for hiding this comment

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

The methods in the class are very huge. Either modularize or add good java docs.

shubhamjainflipkart and others added 22 commits April 4, 2018 14:09
FDG-3059 | Fix to enable Tez jobs processing in dr-elephant
 FDG-3134 | Fixed performance and too many open connections issue
FDG-3906: support to search job with PID and EID
FDG-3906: Pulling dependencies from Fk Repository
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.

5 participants