Skip to content

Commit

Permalink
Merge pull request #62 from DarinJ/checkstyle
Browse files Browse the repository at this point in the history
Added a reasonable maven checkstyle that should enforce nice code.  I…
  • Loading branch information
brndnmtthws committed May 25, 2015
2 parents b7a8278 + 1b0df6b commit c972174
Show file tree
Hide file tree
Showing 7 changed files with 253 additions and 34 deletions.
200 changes: 200 additions & 0 deletions checkstyle.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
"-//Puppy Crawl//DTD Check Configuration 1.2//EN"
"http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
<!--
Checkstyle configuration that checks a modified sun coding conventions from:
- the Java Language Specification at
http://java.sun.com/docs/books/jls/second_edition/html/index.html
- the Sun Code Conventions at http://java.sun.com/docs/codeconv/
- the JDK Api documentation http://java.sun.com/j2se/docs/api/index.html
- some best practices
Checkstyle is very configurable. Be sure to read the documentation at
http://checkstyle.sf.net (or in your downloaded distribution).
Most Checks are configurable, be sure to consult the documentation.
To completely disable a check, just comment it out or delete it from the file.
Finally, it is worth reading the documentation.
-->

<module name="Checker">
<!--
If you set the basedir property below, then all reported file
names will be relative to the specified directory. See
http://checkstyle.sourceforge.net/5.x/config.html#Checker
<property name="basedir" value="${basedir}"/>
-->
<!--Todo(DarinJ): Uncomment to have warnings instead of errors (Build Success, even if failed checkstyles) -->
<!--<property name="severity" value="warning"/>-->

<!-- Checks whether files end with a new line. -->
<!-- See http://checkstyle.sf.net/config_misc.html#NewlineAtEndOfFile -->
<module name="NewlineAtEndOfFile"/>

<!-- Checks that property files contain the same keys. -->
<!-- See http://checkstyle.sf.net/config_misc.html#Translation -->
<module name="Translation"/>

<module name="FileLength"/>

<!-- Following interprets the header file as regular expressions. -->
<!-- <module name="RegexpHeader"/> -->

<module name="FileTabCharacter">
<property name="eachLine" value="true"/>
</module>

<module name="RegexpSingleline">
<!-- \s matches whitespace character, $ matches end of line. -->
<property name="format" value="\s+$"/>
<property name="message" value="Line has trailing spaces."/>
</module>

<module name="TreeWalker">

<property name="cacheFile" value="${checkstyle.cache.file}"/>

<!-- required for SuppressWarningsFilter (and other Suppress* rules not used here) -->
<!-- see http://checkstyle.sourceforge.net/config_annotation.html#SuppressWarningsHolder -->
<module name="SuppressWarningsHolder"/>

<!-- Checks for Javadoc comments. -->
<!-- See http://checkstyle.sf.net/config_javadoc.html -->
<!-- <module name="JavadocMethod"/>
<module name="JavadocType"/>
<module name="JavadocVariable"/>
<module name="JavadocStyle"/> -->


<!-- Checks for Naming Conventions. -->
<!-- See http://checkstyle.sf.net/config_naming.html -->
<module name="ConstantName"/>
<module name="LocalFinalVariableName"/>
<module name="LocalVariableName"/>
<module name="MemberName"/>
<module name="MethodName"/>
<module name="PackageName"/>
<module name="ParameterName"/>
<module name="StaticVariableName"/>
<module name="TypeName"/>


<!-- Checks for Headers -->
<!-- See http://checkstyle.sf.net/config_header.html -->
<!-- <module name="Header"> -->
<!-- The follow property value demonstrates the ability -->
<!-- to have access to ANT properties. In this case it uses -->
<!-- the ${basedir} property to allow Checkstyle to be run -->
<!-- from any directory within a project. See property -->
<!-- expansion, -->
<!-- http://checkstyle.sf.net/config.html#properties -->
<!-- <property -->
<!-- name="headerFile" -->
<!-- value="${basedir}/java.header"/> -->
<!-- </module> -->


<!-- Checks for imports -->
<!-- See http://checkstyle.sf.net/config_import.html -->
<!-- Todo(DarinJ): add back AvoidStarImport -->
<!-- <module name="AvoidStarImport"/> -->
<module name="IllegalImport"/> <!-- defaults to sun.* packages -->
<module name="RedundantImport"/>
<module name="UnusedImports"/>


<!-- Checks for Size Violations. -->
<!-- See http://checkstyle.sf.net/config_sizes.html -->
<!-- Todo(DarinJ): add back LineLength -->
<!-- <module name="LineLength"/> -->
<!-- Todo(DarinJ): add back MethodLength -->
<!-- <module name="MethodLength"/> -->
<module name="ParameterNumber"/>


<!-- Checks for whitespace -->
<!-- See http://checkstyle.sf.net/config_whitespace.html -->
<module name="EmptyForIteratorPad"/>
<module name="MethodParamPad"/>
<module name="NoWhitespaceAfter"/>
<module name="NoWhitespaceBefore"/>
<!-- <module name="OperatorWrap"/> -->
<module name="ParenPad"/>
<module name="TypecastParenPad"/>
<module name="WhitespaceAfter"/>
<module name="WhitespaceAround"/>

<!--Indentation based off google styleguide -->
<module name="Indentation">
<property name="basicOffset" value="2"/>
<property name="braceAdjustment" value="0"/>
<property name="caseIndent" value="2"/>
<property name="throwsIndent" value="4"/>
<property name="lineWrappingIndentation" value="4"/>
<property name="arrayInitIndent" value="2"/>
</module>

<!-- Modifier Checks -->
<!-- See http://checkstyle.sf.net/config_modifiers.html -->
<module name="ModifierOrder"/>
<module name="RedundantModifier"/>

<!-- Checks for blocks. You know, those {}'s -->
<!-- See http://checkstyle.sf.net/config_blocks.html -->
<module name="AvoidNestedBlocks"/>
<module name="EmptyBlock"/>
<module name="LeftCurly"/>
<module name="NeedBraces"/>
<module name="RightCurly"/>

<!-- Checks for common coding problems -->
<!-- See http://checkstyle.sf.net/config_coding.html -->
<!-- Todo(DarinJ): add back AvoidInlineConditional??? used well maybe leave? -->
<!-- <module name="AvoidInlineConditionals"/> -->
<module name="EmptyStatement"/>
<module name="EqualsHashCode"/>
<!-- <module name="HiddenField"/> -->
<module name="IllegalInstantiation"/>
<module name="InnerAssignment"/>
<!-- Todo(DarinJ): add back MagicNumbe -->
<!-- <module name="MagicNumber"/> -->
<module name="MissingSwitchDefault"/>
<module name="SimplifyBooleanExpression"/>
<module name="SimplifyBooleanReturn"/>

<!-- Checks for class design -->
<!-- See http://checkstyle.sf.net/config_design.html -->
<!--<module name="DesignForExtension"/> -->
<module name="FinalClass"/>
<module name="HideUtilityClassConstructor"/>
<module name="InterfaceIsType"/>
<!-- <module name="VisibilityModifier"/> -->

<!-- Miscellaneous other checks. -->
<!-- See http://checkstyle.sf.net/config_misc.html -->
<module name="ArrayTypeStyle"/>
<!--<module name="FinalParameters"/>-->
<module name="TodoComment"/>
<module name="UpperEll"/>

</module>

<!-- Support @SuppressWarnings (added in Checkstyle 5.7) -->
<!-- see http://checkstyle.sourceforge.net/config.html#SuppressWarningsFilter -->
<module name="SuppressWarningsFilter"/>

<!-- Checks properties file for a duplicated properties. -->
<!-- See http://checkstyle.sourceforge.net/config_misc.html#UniqueProperties -->
<module name="UniqueProperties"/>

</module>
20 changes: 20 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,26 @@
<configuration>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
<version>2.15</version>
<executions>
<execution>
<id>validate</id>
<phase>validate</phase>
<configuration>
<configLocation>checkstyle.xml</configLocation>
<encoding>UTF-8</encoding>
<consoleOutput>true</consoleOutput>
<failsOnError>true</failsOnError>
</configuration>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>

Expand Down
3 changes: 1 addition & 2 deletions src/main/java/org/apache/hadoop/mapred/MesosExecutor.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import java.io.*;

import java.lang.reflect.Field;
import java.lang.ReflectiveOperationException;

import java.util.concurrent.BlockingQueue;
import java.util.concurrent.Executors;
Expand Down Expand Up @@ -192,7 +191,7 @@ private JobConf configure(final TaskInfo task) {

/**
* This is a hack to overcome lack of accessibility of the launcher. Will solicit feedback from Hadoop list.
*
*
* @param tracker tracker with launcher we want to kill
* @param name name of the field containing the launcher
* @throws NoSuchFieldException
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/org/apache/hadoop/mapred/MesosTracker.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ public void run() {
// fail to mark some TaskTrackers as active even though they are.
// Here we do a final check with the JobTracker to make sure this
// TaskTracker is really not there before we kill it.
final Collection<TaskTrackerStatus> taskTrackers =
MesosTracker.this.scheduler.jobTracker.taskTrackers();
final Collection<TaskTrackerStatus> taskTrackers = MesosTracker.this.scheduler.jobTracker.taskTrackers();

for (TaskTrackerStatus status : taskTrackers) {
HttpHost host = new HttpHost(status.getHost(), status.getHttpPort());
Expand Down
18 changes: 9 additions & 9 deletions src/main/java/org/apache/hadoop/mapred/ResourcePolicy.java
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public void resourceOffers(SchedulerDriver schedulerDriver, List<Offer> offers)
continue;
}
while (begin <= end && ports.size() < 2) {
int port = begin + (int)(Math.random() * ((end - begin) + 1));
int port = begin + (int) (Math.random() * ((end - begin) + 1));
ports.add(port);
begin += 1;
}
Expand Down Expand Up @@ -220,7 +220,7 @@ public void resourceOffers(SchedulerDriver schedulerDriver, List<Offer> offers)

String jvmOpts = scheduler.conf.get("mapred.mesos.executor.jvm.opts");
if (jvmOpts == null) {
jvmOpts = StringUtils.join(" ", defaultJvmOpts);
jvmOpts = StringUtils.join(" ", defaultJvmOpts);
}

// Set up the environment for running the TaskTracker.
Expand All @@ -232,8 +232,8 @@ public void resourceOffers(SchedulerDriver schedulerDriver, List<Offer> offers)
.setValue(
jvmOpts +
" -Xmx" + tasktrackerJVMHeap + "m" +
" -XX:NewSize=" + tasktrackerJVMHeap / 3 + "m -XX:MaxNewSize=" + (int)Math.floor
(tasktrackerJVMHeap * 0.6) + "m"
" -XX:NewSize=" + tasktrackerJVMHeap / 3 + "m -XX:MaxNewSize=" +
(int) Math.floor(tasktrackerJVMHeap * 0.6) + "m"
));

// Set java specific environment, appropriately.
Expand Down Expand Up @@ -277,7 +277,7 @@ public void resourceOffers(SchedulerDriver schedulerDriver, List<Offer> offers)

directory = new File(uri).getName().split("\\.")[0] + "*";
} else if (!isUriSet) {
LOG.info("mapred.mesos.executor.uri is not set, relying on configured 'mapred.mesos.executor.directory' for working Hadoop distribution");
LOG.info("mapred.mesos.executor.uri is not set, relying on configured 'mapred.mesos.executor.directory' for working Hadoop distribution");
}

String command = scheduler.conf.get("mapred.mesos.executor.command");
Expand All @@ -290,7 +290,7 @@ public void resourceOffers(SchedulerDriver schedulerDriver, List<Offer> offers)
.setEnvironment(envBuilder)
.setValue(String.format("cd %s && %s", directory, command));
if (uri != null) {
commandInfo.addUris(CommandInfo.URI.newBuilder().setValue(uri));
commandInfo.addUris(CommandInfo.URI.newBuilder().setValue(uri));
}

// Populate ContainerInfo if needed
Expand Down Expand Up @@ -518,8 +518,8 @@ public void computeNeededSlots(List<JobInProgress> jobsInProgress,
" Needed Reduce Slots: " + neededReduceSlots,
" Unhealthy Trackers: " + unhealthyTrackers)));

File stateFile = scheduler.stateFile;
if (stateFile != null) {
File stateFile = scheduler.stateFile;
if (stateFile != null) {
// Update state file
synchronized (this) {
Set<String> hosts = new HashSet<>();
Expand All @@ -546,7 +546,7 @@ public void computeNeededSlots(List<JobInProgress> jobsInProgress,
"")));
fstream.close();
if (!tmp.renameTo(stateFile)) {
LOG.error("Can't overwrite state " + stateFile.getAbsolutePath());
LOG.error("Can't overwrite state " + stateFile.getAbsolutePath());
}
} catch (Exception e) {
LOG.error("Can't write state file: " + e.getMessage());
Expand Down
37 changes: 17 additions & 20 deletions src/main/java/org/apache/mesos/hadoop/Metrics.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,10 @@
public class Metrics {
public MetricRegistry registry;
public Meter killMeter, flakyTrackerKilledMeter, launchTimeout, periodicGC;
public Map<Integer, Meter> jobStateMeter =
new ConcurrentHashMap<Integer, Meter>();
public Map<TaskState, Meter> taskStateMeter =
new ConcurrentHashMap<TaskState, Meter>();
public Map<Integer, Meter> jobStateMeter = new ConcurrentHashMap<Integer, Meter>();
public Map<TaskState, Meter> taskStateMeter = new ConcurrentHashMap<TaskState, Meter>();
public com.codahale.metrics.Timer jobTimer, trackerTimer;
public Map<JobID, com.codahale.metrics.Timer.Context> jobTimerContexts =
new ConcurrentHashMap<JobID, com.codahale.metrics.Timer.Context>();
public Map<JobID, com.codahale.metrics.Timer.Context> jobTimerContexts = new ConcurrentHashMap<JobID, com.codahale.metrics.Timer.Context>();

public Metrics(Configuration conf) {
registry = new MetricRegistry();
Expand Down Expand Up @@ -62,10 +59,10 @@ public Metrics(Configuration conf) {
final int interval = conf.getInt("mapred.mesos.metrics.csv.interval", 60);

CsvReporter csvReporter = CsvReporter.forRegistry(registry)
.convertRatesTo(TimeUnit.SECONDS)
.convertDurationsTo(TimeUnit.MILLISECONDS)
.filter(MetricFilter.ALL)
.build(new File(path));
.convertRatesTo(TimeUnit.SECONDS)
.convertDurationsTo(TimeUnit.MILLISECONDS)
.filter(MetricFilter.ALL)
.build(new File(path));
csvReporter.start(interval, TimeUnit.SECONDS);
}

Expand All @@ -78,11 +75,11 @@ public Metrics(Configuration conf) {

Graphite graphite = new Graphite(new InetSocketAddress(host, port));
GraphiteReporter graphiteReporter = GraphiteReporter.forRegistry(registry)
.prefixedWith(prefix)
.convertRatesTo(TimeUnit.SECONDS)
.convertDurationsTo(TimeUnit.MILLISECONDS)
.filter(MetricFilter.ALL)
.build(graphite);
.prefixedWith(prefix)
.convertRatesTo(TimeUnit.SECONDS)
.convertDurationsTo(TimeUnit.MILLISECONDS)
.filter(MetricFilter.ALL)
.build(graphite);
graphiteReporter.start(interval, TimeUnit.SECONDS);
}

Expand All @@ -106,11 +103,11 @@ public Metrics(Configuration conf) {
consistency);

CassandraReporter cassandraReporter = CassandraReporter.forRegistry(registry)
.prefixedWith(prefix)
.convertRatesTo(TimeUnit.SECONDS)
.convertDurationsTo(TimeUnit.MILLISECONDS)
.filter(MetricFilter.ALL)
.build(cassandra);
.prefixedWith(prefix)
.convertRatesTo(TimeUnit.SECONDS)
.convertDurationsTo(TimeUnit.MILLISECONDS)
.filter(MetricFilter.ALL)
.build(cassandra);
cassandraReporter.start(interval, TimeUnit.SECONDS);
}
}
Expand Down

0 comments on commit c972174

Please sign in to comment.