Skip to content

Commit 325bcf4

Browse files
committed
various PR fixes (checkstyle, spotbugs)
1 parent 3fa0220 commit 325bcf4

8 files changed

Lines changed: 103 additions & 60 deletions

File tree

tez-api/findbugs-exclude.xml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,4 +131,16 @@
131131
<Bug pattern="IS2_INCONSISTENT_SYNC" />
132132
</Match>
133133

134+
<!-- TEZ-4008 -->
135+
<Match>
136+
<Class name="org.apache.tez.client.registry.AMRecord" />
137+
<Method name="&lt;init&gt;" params="org.apache.hadoop.registry.client.types.ServiceRecord"/>
138+
<Bug pattern="CT_CONSTRUCTOR_THROW" />
139+
</Match>
140+
141+
<Match>
142+
<Class name="org.apache.tez.client.registry.AMRecord" />
143+
<Method name="&lt;init&gt;" params="org.apache.hadoop.yarn.api.records.ApplicationId,java.lang.String,java.lang.Integer,java.lang.String"/>
144+
<Bug pattern="EI_EXPOSE_REP2" />
145+
</Match>
134146
</FindBugsFilter>

tez-api/src/main/java/org/apache/tez/client/registry/AMRecord.java

Lines changed: 25 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,23 @@
2222
import org.apache.hadoop.registry.client.types.ServiceRecord;
2323
import org.apache.hadoop.yarn.api.records.ApplicationId;
2424

25-
import com.google.common.base.Preconditions;
2625

2726
/**
28-
* Represents an instance of an AM (DAGClientServer) in the AM registry
27+
* Represents an instance of an AM (DAGClientServer) in the AM registry.
2928
*/
3029
@InterfaceAudience.Public
3130
public class AMRecord {
32-
private ApplicationId appId;
33-
private String host;
34-
private int port;
35-
private String id;
36-
private final static String APP_ID_RECORD_KEY = "appId";
37-
private final static String HOST_RECORD_KEY = "host";
38-
private final static String PORT_RECORD_KEY = "port";
39-
private final static String OPAQUE_ID_KEY = "id";
31+
private static final String APP_ID_RECORD_KEY = "appId";
32+
private static final String HOST_RECORD_KEY = "host";
33+
private static final String PORT_RECORD_KEY = "port";
34+
private static final String OPAQUE_ID_KEY = "id";
35+
36+
private final ApplicationId appId;
37+
private final String host;
38+
private final int port;
39+
private final String id;
4040

4141
public AMRecord(ApplicationId appId, String host, int port, String id) {
42-
Preconditions.checkNotNull(appId);
43-
Preconditions.checkNotNull(host);
4442
this.appId = appId;
4543
this.host = host;
4644
this.port = port;
@@ -49,29 +47,21 @@ public AMRecord(ApplicationId appId, String host, int port, String id) {
4947
}
5048

5149
public AMRecord(AMRecord other) {
52-
Preconditions.checkNotNull(other);
53-
this.appId = other.getApplicationId();
50+
this.appId = ApplicationId.fromString(other.getApplicationId());
5451
this.host = other.getHost();
5552
this.port = other.getPort();
5653
this.id = other.getId();
5754
}
5855

5956
public AMRecord(ServiceRecord serviceRecord) {
60-
String serviceAppId = serviceRecord.get(APP_ID_RECORD_KEY);
61-
Preconditions.checkNotNull(serviceAppId);
62-
this.appId = ApplicationId.fromString(serviceAppId);
63-
String serviceHost = serviceRecord.get(HOST_RECORD_KEY);
64-
Preconditions.checkNotNull(serviceHost);
65-
this.host = serviceHost;
66-
String servicePort = serviceRecord.get(PORT_RECORD_KEY);
67-
this.port = Integer.parseInt(servicePort);
68-
String serviceId = serviceRecord.get(OPAQUE_ID_KEY);
69-
Preconditions.checkNotNull(serviceId);
70-
this.id = serviceId;
57+
this.appId = ApplicationId.fromString(serviceRecord.get(APP_ID_RECORD_KEY));
58+
this.host = serviceRecord.get(HOST_RECORD_KEY);
59+
this.port = Integer.parseInt(serviceRecord.get(PORT_RECORD_KEY));
60+
this.id = serviceRecord.get(OPAQUE_ID_KEY);
7161
}
7262

73-
public ApplicationId getApplicationId() {
74-
return appId;
63+
public String getApplicationId() {
64+
return appId.toString();
7565
}
7666

7767
public String getHost() {
@@ -82,12 +72,13 @@ public int getPort() {
8272
return port;
8373
}
8474

85-
public String getId() { return id; }
75+
public String getId() {
76+
return id;
77+
}
8678

8779
@Override
8880
public boolean equals(Object other) {
89-
if(other instanceof AMRecord) {
90-
AMRecord otherRecord = (AMRecord) other;
81+
if (other instanceof AMRecord otherRecord) {
9182
return appId.equals(otherRecord.appId)
9283
&& host.equals(otherRecord.host)
9384
&& port == otherRecord.port
@@ -97,11 +88,6 @@ public boolean equals(Object other) {
9788
}
9889
}
9990

100-
@Override
101-
public int hashCode() {
102-
return appId.hashCode() * host.hashCode() * id.hashCode() + port;
103-
}
104-
10591
public ServiceRecord toServiceRecord() {
10692
ServiceRecord serviceRecord = new ServiceRecord();
10793
serviceRecord.set(APP_ID_RECORD_KEY, appId);
@@ -111,4 +97,8 @@ public ServiceRecord toServiceRecord() {
11197
return serviceRecord;
11298
}
11399

100+
@Override
101+
public int hashCode() {
102+
return appId.hashCode() * host.hashCode() * id.hashCode() + port;
103+
}
114104
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
@Public
20+
@Evolving
21+
package org.apache.tez.client.registry;
22+
23+
import org.apache.hadoop.classification.InterfaceAudience.Public;
24+
import org.apache.hadoop.classification.InterfaceStability.Evolving;

tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2353,7 +2353,7 @@ static Set<String> getPropertySet() {
23532353
/**
23542354
* String value. The class to be used for the AM registry.
23552355
*/
2356-
@ConfigurationScope(Scope.AM)
2357-
@ConfigurationProperty
2358-
public static final String TEZ_AM_REGISTRY_CLASS = TEZ_AM_PREFIX + "registry.class";
2356+
@ConfigurationScope(Scope.AM)
2357+
@ConfigurationProperty
2358+
public static final String TEZ_AM_REGISTRY_CLASS = TEZ_AM_PREFIX + "registry.class";
23592359
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
@Public
20+
@Evolving
21+
package org.apache.tez.dag.api.client.registry;
22+
23+
import org.apache.hadoop.classification.InterfaceAudience.Public;
24+
import org.apache.hadoop.classification.InterfaceStability.Evolving;

tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -670,14 +670,12 @@ protected void initClientRpcServer() {
670670
}
671671

672672
@VisibleForTesting
673-
public static void initAmRegistry(ApplicationId appId, String amUUID, AMRegistry amRegistry, DAGClientServer dagClientServer) throws Exception {
673+
public static void initAmRegistry(ApplicationId appId, String amUUID, AMRegistry amRegistry,
674+
DAGClientServer dagClientServer) {
674675
if(amRegistry != null) {
675676
dagClientServer.registerServiceListener((service) -> {
676677
if (service.isInState(STATE.STARTED)) {
677-
AMRecord amRecord = AMRegistryUtils.recordForDAGClientServer(
678-
appId,
679-
amUUID,
680-
dagClientServer);
678+
AMRecord amRecord = AMRegistryUtils.recordForDAGClientServer(appId, amUUID, dagClientServer);
681679
try {
682680
amRegistry.add(amRecord);
683681
} catch (Exception e) {

tez-dag/src/main/java/org/apache/tez/dag/utils/AMRegistryUtils.java

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,18 @@
2828
import org.apache.tez.dag.api.client.DAGClientServer;
2929
import org.apache.tez.dag.api.client.registry.AMRegistry;
3030

31-
public class AMRegistryUtils {
32-
public static AMRecord recordForDAGClientServer(ApplicationId appId, String opaqueId, DAGClientServer dagClientServer) {
31+
public final class AMRegistryUtils {
32+
33+
private AMRegistryUtils() {}
34+
35+
public static AMRecord recordForDAGClientServer(ApplicationId appId, String opaqueId,
36+
DAGClientServer dagClientServer) {
3337
InetSocketAddress address = dagClientServer.getBindAddress();
3438
return new AMRecord(appId, address.getHostName(), address.getPort(), opaqueId);
3539
}
3640

3741
public static AMRegistry createAMRegistry(Configuration conf) throws Exception {
3842
String tezAMRegistryClass = conf.get(TezConfiguration.TEZ_AM_REGISTRY_CLASS);
39-
if(tezAMRegistryClass == null) {
40-
return null;
41-
} else {
42-
AMRegistry amRegistry = ReflectionUtils.createClazzInstance(tezAMRegistryClass);
43-
return amRegistry;
44-
}
43+
return tezAMRegistryClass == null ? null : ReflectionUtils.createClazzInstance(tezAMRegistryClass);
4544
}
4645
}

tez-dag/src/test/java/org/apache/tez/dag/api/client/registry/TestAMRegistry.java

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public void testAMRegistryFactory() throws Exception {
4848
String className = "org.apache.tez.dag.api.client.registry.TestAMRegistry$SkeletonAMRegistry";
4949
conf.set(TezConfiguration.TEZ_AM_REGISTRY_CLASS, className);
5050
amRegistry = AMRegistryUtils.createAMRegistry(conf);
51-
assertEquals(amRegistry.getClass().getName(), className);
51+
assertEquals(className, amRegistry.getClass().getName());
5252
}
5353

5454
@Test(timeout = 5000)
@@ -57,14 +57,10 @@ public void testRecordForDagServer() {
5757
when(dagClientServer.getBindAddress()).thenReturn(new InetSocketAddress("testhost", 1000));
5858
ApplicationId appId = ApplicationId.newInstance(0, 1);
5959
String id = UUID.randomUUID().toString();
60-
AMRecord record = AMRegistryUtils.recordForDAGClientServer(
61-
appId,
62-
id,
63-
dagClientServer
64-
);
65-
assertEquals(record.getApplicationId(), appId);
66-
assertEquals(record.getHost(), "testhost");
67-
assertEquals(record.getPort(), 1000);
60+
AMRecord record = AMRegistryUtils.recordForDAGClientServer(appId, id, dagClientServer);
61+
assertEquals(appId.toString(), record.getApplicationId());
62+
assertEquals("testhost", record.getHost());
63+
assertEquals(1000, record.getPort());
6864
assertEquals(record.getId(), id);
6965
}
7066

0 commit comments

Comments
 (0)