You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by "paul sutter (JIRA)" <ji...@apache.org> on 2006/04/20 00:18:34 UTC
[jira] Created: (HADOOP-151) Misdeclared field in RPC.class
Misdeclared field in RPC.class
------------------------------
Key: HADOOP-151
URL: http://issues.apache.org/jira/browse/HADOOP-151
Project: Hadoop
Type: Bug
Reporter: paul sutter
In RPC.java, the field named CLIENT should be neither static, nor a field of RPC. It should be (a) a private nonstatic field of InvocationHandler(),and (just further down), (b) a local variable in the RPC.call() method below. The comment above the declaration was a bit of giveaway:
//TODO mb@media-style.com: static client or non-static client?
private static Client CLIENT;
private static class Invoker implements InvocationHandler {
private InetSocketAddress address;
public Invoker(InetSocketAddress address, Configuration conf) {
this.address = address;
CLIENT = (Client) conf.getObject(Client.class.getName());
if(CLIENT == null) {
CLIENT = new Client(ObjectWritable.class, conf);
conf.setObject(Client.class.getName(), CLIENT);
}
}
public Object invoke(Object proxy, Method method, Object[] args)
throws Throwable {
ObjectWritable value = (ObjectWritable)
CLIENT.call(new Invocation(method, args), address);
return value.get();
}
}
/** Construct a client-side proxy object that implements the named protocol,
* talking to a server at the named address. */
public static Object getProxy(Class protocol, InetSocketAddress addr, Configuration conf) {
return Proxy.newProxyInstance(protocol.getClassLoader(),
new Class[] { protocol },
new Invoker(addr, conf));
}
/** Expert: Make multiple, parallel calls to a set of servers. */
public static Object[] call(Method method, Object[][] params,
InetSocketAddress[] addrs, Configuration conf)
throws IOException {
Invocation[] invocations = new Invocation[params.length];
for (int i = 0; i < params.length; i++)
invocations[i] = new Invocation(method, params[i]);
CLIENT = (Client) conf.getObject(Client.class.getName());
if(CLIENT == null) {
CLIENT = new Client(ObjectWritable.class, conf);
conf.setObject(Client.class.getName(), CLIENT);
}
Writable[] wrappedValues = CLIENT.call(invocations, addrs);
if (method.getReturnType() == Void.TYPE) {
return null;
}
Object[] values =
(Object[])Array.newInstance(method.getReturnType(),wrappedValues.length);
for (int i = 0; i < values.length; i++)
if (wrappedValues[i] != null)
values[i] = ((ObjectWritable)wrappedValues[i]).get();
return values;
}.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (HADOOP-151) Misdeclared field in RPC.class
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/HADOOP-151?page=comments#action_12375205 ]
Doug Cutting commented on HADOOP-151:
-------------------------------------
What's the problem with a static CLIENT? What problems does this cause? The client has the connection pool, so one potential problem is that, a large request or response will delay other requests or responses to/from the same host. Is that the issue you're seeing? If so, can you provide more details about the circumstances where this occurs?
> Misdeclared field in RPC.class
> ------------------------------
>
> Key: HADOOP-151
> URL: http://issues.apache.org/jira/browse/HADOOP-151
> Project: Hadoop
> Type: Bug
> Reporter: paul sutter
>
> In RPC.java, the field named CLIENT should be neither static, nor a field of RPC. It should be (a) a private nonstatic field of InvocationHandler(),and (just further down), (b) a local variable in the RPC.call() method below. The comment above the declaration was a bit of giveaway:
> //TODO mb@media-style.com: static client or non-static client?
> private static Client CLIENT;
> private static class Invoker implements InvocationHandler {
> private InetSocketAddress address;
> public Invoker(InetSocketAddress address, Configuration conf) {
> this.address = address;
> CLIENT = (Client) conf.getObject(Client.class.getName());
> if(CLIENT == null) {
> CLIENT = new Client(ObjectWritable.class, conf);
> conf.setObject(Client.class.getName(), CLIENT);
> }
> }
> public Object invoke(Object proxy, Method method, Object[] args)
> throws Throwable {
> ObjectWritable value = (ObjectWritable)
> CLIENT.call(new Invocation(method, args), address);
> return value.get();
> }
> }
> /** Construct a client-side proxy object that implements the named protocol,
> * talking to a server at the named address. */
> public static Object getProxy(Class protocol, InetSocketAddress addr, Configuration conf) {
> return Proxy.newProxyInstance(protocol.getClassLoader(),
> new Class[] { protocol },
> new Invoker(addr, conf));
> }
> /** Expert: Make multiple, parallel calls to a set of servers. */
> public static Object[] call(Method method, Object[][] params,
> InetSocketAddress[] addrs, Configuration conf)
> throws IOException {
> Invocation[] invocations = new Invocation[params.length];
> for (int i = 0; i < params.length; i++)
> invocations[i] = new Invocation(method, params[i]);
> CLIENT = (Client) conf.getObject(Client.class.getName());
> if(CLIENT == null) {
> CLIENT = new Client(ObjectWritable.class, conf);
> conf.setObject(Client.class.getName(), CLIENT);
> }
> Writable[] wrappedValues = CLIENT.call(invocations, addrs);
>
> if (method.getReturnType() == Void.TYPE) {
> return null;
> }
> Object[] values =
> (Object[])Array.newInstance(method.getReturnType(),wrappedValues.length);
> for (int i = 0; i < values.length; i++)
> if (wrappedValues[i] != null)
> values[i] = ((ObjectWritable)wrappedValues[i]).get();
>
> return values;
> }.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Updated: (HADOOP-151) RPC code has socket leak?
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/HADOOP-151?page=all ]
Doug Cutting updated HADOOP-151:
--------------------------------
Summary: RPC code has socket leak? (was: Misdeclared field in RPC.class)
Component: ipc
Fix Version: 0.2
Version: 0.1.1
0.1.0
0.2
Description:
In RPC.java, the field named CLIENT should be neither static, nor a field of RPC. It should be (a) a private nonstatic field of InvocationHandler(),and (just further down), (b) a local variable in the RPC.call() method below. The comment above the declaration was a bit of giveaway:
//TODO mb@media-style.com: static client or non-static client?
private static Client CLIENT;
private static class Invoker implements InvocationHandler {
private InetSocketAddress address;
public Invoker(InetSocketAddress address, Configuration conf) {
this.address = address;
CLIENT = (Client) conf.getObject(Client.class.getName());
if(CLIENT == null) {
CLIENT = new Client(ObjectWritable.class, conf);
conf.setObject(Client.class.getName(), CLIENT);
}
}
public Object invoke(Object proxy, Method method, Object[] args)
throws Throwable {
ObjectWritable value = (ObjectWritable)
CLIENT.call(new Invocation(method, args), address);
return value.get();
}
}
/** Construct a client-side proxy object that implements the named protocol,
* talking to a server at the named address. */
public static Object getProxy(Class protocol, InetSocketAddress addr, Configuration conf) {
return Proxy.newProxyInstance(protocol.getClassLoader(),
new Class[] { protocol },
new Invoker(addr, conf));
}
/** Expert: Make multiple, parallel calls to a set of servers. */
public static Object[] call(Method method, Object[][] params,
InetSocketAddress[] addrs, Configuration conf)
throws IOException {
Invocation[] invocations = new Invocation[params.length];
for (int i = 0; i < params.length; i++)
invocations[i] = new Invocation(method, params[i]);
CLIENT = (Client) conf.getObject(Client.class.getName());
if(CLIENT == null) {
CLIENT = new Client(ObjectWritable.class, conf);
conf.setObject(Client.class.getName(), CLIENT);
}
Writable[] wrappedValues = CLIENT.call(invocations, addrs);
if (method.getReturnType() == Void.TYPE) {
return null;
}
Object[] values =
(Object[])Array.newInstance(method.getReturnType(),wrappedValues.length);
for (int i = 0; i < values.length; i++)
if (wrappedValues[i] != null)
values[i] = ((ObjectWritable)wrappedValues[i]).get();
return values;
}.
was:
In RPC.java, the field named CLIENT should be neither static, nor a field of RPC. It should be (a) a private nonstatic field of InvocationHandler(),and (just further down), (b) a local variable in the RPC.call() method below. The comment above the declaration was a bit of giveaway:
//TODO mb@media-style.com: static client or non-static client?
private static Client CLIENT;
private static class Invoker implements InvocationHandler {
private InetSocketAddress address;
public Invoker(InetSocketAddress address, Configuration conf) {
this.address = address;
CLIENT = (Client) conf.getObject(Client.class.getName());
if(CLIENT == null) {
CLIENT = new Client(ObjectWritable.class, conf);
conf.setObject(Client.class.getName(), CLIENT);
}
}
public Object invoke(Object proxy, Method method, Object[] args)
throws Throwable {
ObjectWritable value = (ObjectWritable)
CLIENT.call(new Invocation(method, args), address);
return value.get();
}
}
/** Construct a client-side proxy object that implements the named protocol,
* talking to a server at the named address. */
public static Object getProxy(Class protocol, InetSocketAddress addr, Configuration conf) {
return Proxy.newProxyInstance(protocol.getClassLoader(),
new Class[] { protocol },
new Invoker(addr, conf));
}
/** Expert: Make multiple, parallel calls to a set of servers. */
public static Object[] call(Method method, Object[][] params,
InetSocketAddress[] addrs, Configuration conf)
throws IOException {
Invocation[] invocations = new Invocation[params.length];
for (int i = 0; i < params.length; i++)
invocations[i] = new Invocation(method, params[i]);
CLIENT = (Client) conf.getObject(Client.class.getName());
if(CLIENT == null) {
CLIENT = new Client(ObjectWritable.class, conf);
conf.setObject(Client.class.getName(), CLIENT);
}
Writable[] wrappedValues = CLIENT.call(invocations, addrs);
if (method.getReturnType() == Void.TYPE) {
return null;
}
Object[] values =
(Object[])Array.newInstance(method.getReturnType(),wrappedValues.length);
for (int i = 0; i < values.length; i++)
if (wrappedValues[i] != null)
values[i] = ((ObjectWritable)wrappedValues[i]).get();
return values;
}.
Assign To: Doug Cutting
> RPC code has socket leak?
> -------------------------
>
> Key: HADOOP-151
> URL: http://issues.apache.org/jira/browse/HADOOP-151
> Project: Hadoop
> Type: Bug
> Components: ipc
> Versions: 0.1.0, 0.2, 0.1.1
> Reporter: paul sutter
> Assignee: Doug Cutting
> Fix For: 0.2
>
> In RPC.java, the field named CLIENT should be neither static, nor a field of RPC. It should be (a) a private nonstatic field of InvocationHandler(),and (just further down), (b) a local variable in the RPC.call() method below. The comment above the declaration was a bit of giveaway:
> //TODO mb@media-style.com: static client or non-static client?
> private static Client CLIENT;
> private static class Invoker implements InvocationHandler {
> private InetSocketAddress address;
> public Invoker(InetSocketAddress address, Configuration conf) {
> this.address = address;
> CLIENT = (Client) conf.getObject(Client.class.getName());
> if(CLIENT == null) {
> CLIENT = new Client(ObjectWritable.class, conf);
> conf.setObject(Client.class.getName(), CLIENT);
> }
> }
> public Object invoke(Object proxy, Method method, Object[] args)
> throws Throwable {
> ObjectWritable value = (ObjectWritable)
> CLIENT.call(new Invocation(method, args), address);
> return value.get();
> }
> }
> /** Construct a client-side proxy object that implements the named protocol,
> * talking to a server at the named address. */
> public static Object getProxy(Class protocol, InetSocketAddress addr, Configuration conf) {
> return Proxy.newProxyInstance(protocol.getClassLoader(),
> new Class[] { protocol },
> new Invoker(addr, conf));
> }
> /** Expert: Make multiple, parallel calls to a set of servers. */
> public static Object[] call(Method method, Object[][] params,
> InetSocketAddress[] addrs, Configuration conf)
> throws IOException {
> Invocation[] invocations = new Invocation[params.length];
> for (int i = 0; i < params.length; i++)
> invocations[i] = new Invocation(method, params[i]);
> CLIENT = (Client) conf.getObject(Client.class.getName());
> if(CLIENT == null) {
> CLIENT = new Client(ObjectWritable.class, conf);
> conf.setObject(Client.class.getName(), CLIENT);
> }
> Writable[] wrappedValues = CLIENT.call(invocations, addrs);
>
> if (method.getReturnType() == Void.TYPE) {
> return null;
> }
> Object[] values =
> (Object[])Array.newInstance(method.getReturnType(),wrappedValues.length);
> for (int i = 0; i < values.length; i++)
> if (wrappedValues[i] != null)
> values[i] = ((ObjectWritable)wrappedValues[i]).get();
>
> return values;
> }.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (HADOOP-151) Misdeclared field in RPC.class
Posted by "paul sutter (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/HADOOP-151?page=comments#action_12375209 ]
paul sutter commented on HADOOP-151:
------------------------------------
If thre is only one Client object, then you're right, its OK that its static.
The fact that the object is also stored in the conf - combined with the comment - made it look like a coding error (since its not accessed by anything but those parts of the code).
> Misdeclared field in RPC.class
> ------------------------------
>
> Key: HADOOP-151
> URL: http://issues.apache.org/jira/browse/HADOOP-151
> Project: Hadoop
> Type: Bug
> Reporter: paul sutter
>
> In RPC.java, the field named CLIENT should be neither static, nor a field of RPC. It should be (a) a private nonstatic field of InvocationHandler(),and (just further down), (b) a local variable in the RPC.call() method below. The comment above the declaration was a bit of giveaway:
> //TODO mb@media-style.com: static client or non-static client?
> private static Client CLIENT;
> private static class Invoker implements InvocationHandler {
> private InetSocketAddress address;
> public Invoker(InetSocketAddress address, Configuration conf) {
> this.address = address;
> CLIENT = (Client) conf.getObject(Client.class.getName());
> if(CLIENT == null) {
> CLIENT = new Client(ObjectWritable.class, conf);
> conf.setObject(Client.class.getName(), CLIENT);
> }
> }
> public Object invoke(Object proxy, Method method, Object[] args)
> throws Throwable {
> ObjectWritable value = (ObjectWritable)
> CLIENT.call(new Invocation(method, args), address);
> return value.get();
> }
> }
> /** Construct a client-side proxy object that implements the named protocol,
> * talking to a server at the named address. */
> public static Object getProxy(Class protocol, InetSocketAddress addr, Configuration conf) {
> return Proxy.newProxyInstance(protocol.getClassLoader(),
> new Class[] { protocol },
> new Invoker(addr, conf));
> }
> /** Expert: Make multiple, parallel calls to a set of servers. */
> public static Object[] call(Method method, Object[][] params,
> InetSocketAddress[] addrs, Configuration conf)
> throws IOException {
> Invocation[] invocations = new Invocation[params.length];
> for (int i = 0; i < params.length; i++)
> invocations[i] = new Invocation(method, params[i]);
> CLIENT = (Client) conf.getObject(Client.class.getName());
> if(CLIENT == null) {
> CLIENT = new Client(ObjectWritable.class, conf);
> conf.setObject(Client.class.getName(), CLIENT);
> }
> Writable[] wrappedValues = CLIENT.call(invocations, addrs);
>
> if (method.getReturnType() == Void.TYPE) {
> return null;
> }
> Object[] values =
> (Object[])Array.newInstance(method.getReturnType(),wrappedValues.length);
> for (int i = 0; i < values.length; i++)
> if (wrappedValues[i] != null)
> values[i] = ((ObjectWritable)wrappedValues[i]).get();
>
> return values;
> }.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (HADOOP-151) Misdeclared field in RPC.class
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/HADOOP-151?page=comments#action_12375212 ]
Doug Cutting commented on HADOOP-151:
-------------------------------------
You're right, that code got uglified by the configuration stuff. There should only need to be a single Client for RPC. Each Client keeps a cache of TCP connections to other host:port pairs expecting the same Writable class for requests and responses. All RPCs use Invocation as the request class and ObjectWritable as the response class, so they can generally share connections.
The problem is that there's no longer a global configuration. So a global client could be lazily constructed the first time a call is made with the invoker's configuration. Or we could create a new client for each configuration. But, you're right, we shouldn't do both, as the current code does. Changing the global CLIENT shouldn't break anything, but it's also silly and wasteful. The only thing the client reads from the configuration is the RPC timeout. It might almost be better to remove the configuration from Client altogether and always explicitly pass the timeout as a parameter to call().
I wonder if this could even be leading to socket leaks? Hmm. A client doesn't close it's connections until it's either explicitly closed (which these are not) or until the remote server dies. So we should probably fix this somehow.
> Misdeclared field in RPC.class
> ------------------------------
>
> Key: HADOOP-151
> URL: http://issues.apache.org/jira/browse/HADOOP-151
> Project: Hadoop
> Type: Bug
> Components: ipc
> Versions: 0.1.0, 0.2, 0.1.1
> Reporter: paul sutter
> Fix For: 0.2
>
> In RPC.java, the field named CLIENT should be neither static, nor a field of RPC. It should be (a) a private nonstatic field of InvocationHandler(),and (just further down), (b) a local variable in the RPC.call() method below. The comment above the declaration was a bit of giveaway:
> //TODO mb@media-style.com: static client or non-static client?
> private static Client CLIENT;
> private static class Invoker implements InvocationHandler {
> private InetSocketAddress address;
> public Invoker(InetSocketAddress address, Configuration conf) {
> this.address = address;
> CLIENT = (Client) conf.getObject(Client.class.getName());
> if(CLIENT == null) {
> CLIENT = new Client(ObjectWritable.class, conf);
> conf.setObject(Client.class.getName(), CLIENT);
> }
> }
> public Object invoke(Object proxy, Method method, Object[] args)
> throws Throwable {
> ObjectWritable value = (ObjectWritable)
> CLIENT.call(new Invocation(method, args), address);
> return value.get();
> }
> }
> /** Construct a client-side proxy object that implements the named protocol,
> * talking to a server at the named address. */
> public static Object getProxy(Class protocol, InetSocketAddress addr, Configuration conf) {
> return Proxy.newProxyInstance(protocol.getClassLoader(),
> new Class[] { protocol },
> new Invoker(addr, conf));
> }
> /** Expert: Make multiple, parallel calls to a set of servers. */
> public static Object[] call(Method method, Object[][] params,
> InetSocketAddress[] addrs, Configuration conf)
> throws IOException {
> Invocation[] invocations = new Invocation[params.length];
> for (int i = 0; i < params.length; i++)
> invocations[i] = new Invocation(method, params[i]);
> CLIENT = (Client) conf.getObject(Client.class.getName());
> if(CLIENT == null) {
> CLIENT = new Client(ObjectWritable.class, conf);
> conf.setObject(Client.class.getName(), CLIENT);
> }
> Writable[] wrappedValues = CLIENT.call(invocations, addrs);
>
> if (method.getReturnType() == Void.TYPE) {
> return null;
> }
> Object[] values =
> (Object[])Array.newInstance(method.getReturnType(),wrappedValues.length);
> for (int i = 0; i < values.length; i++)
> if (wrappedValues[i] != null)
> values[i] = ((ObjectWritable)wrappedValues[i]).get();
>
> return values;
> }.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Resolved: (HADOOP-151) RPC code has socket leak?
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/HADOOP-151?page=all ]
Doug Cutting resolved HADOOP-151:
---------------------------------
Resolution: Fixed
I just fixed this. I restored the original behaviour, where a single client is used for all calls, regardless of the configuration. This results in the use of a single connection pool for all RPC calls, and hence closes a potential socket leak. Long-term we might consider other approaches to pooling connections. Today it seemed more important to close the leak than to, e.g., permit different ipc timeouts for different jobs.
> RPC code has socket leak?
> -------------------------
>
> Key: HADOOP-151
> URL: http://issues.apache.org/jira/browse/HADOOP-151
> Project: Hadoop
> Type: Bug
> Components: ipc
> Versions: 0.1.0, 0.2, 0.1.1
> Reporter: paul sutter
> Assignee: Doug Cutting
> Fix For: 0.2
>
> In RPC.java, the field named CLIENT should be neither static, nor a field of RPC. It should be (a) a private nonstatic field of InvocationHandler(),and (just further down), (b) a local variable in the RPC.call() method below. The comment above the declaration was a bit of giveaway:
> //TODO mb@media-style.com: static client or non-static client?
> private static Client CLIENT;
> private static class Invoker implements InvocationHandler {
> private InetSocketAddress address;
> public Invoker(InetSocketAddress address, Configuration conf) {
> this.address = address;
> CLIENT = (Client) conf.getObject(Client.class.getName());
> if(CLIENT == null) {
> CLIENT = new Client(ObjectWritable.class, conf);
> conf.setObject(Client.class.getName(), CLIENT);
> }
> }
> public Object invoke(Object proxy, Method method, Object[] args)
> throws Throwable {
> ObjectWritable value = (ObjectWritable)
> CLIENT.call(new Invocation(method, args), address);
> return value.get();
> }
> }
> /** Construct a client-side proxy object that implements the named protocol,
> * talking to a server at the named address. */
> public static Object getProxy(Class protocol, InetSocketAddress addr, Configuration conf) {
> return Proxy.newProxyInstance(protocol.getClassLoader(),
> new Class[] { protocol },
> new Invoker(addr, conf));
> }
> /** Expert: Make multiple, parallel calls to a set of servers. */
> public static Object[] call(Method method, Object[][] params,
> InetSocketAddress[] addrs, Configuration conf)
> throws IOException {
> Invocation[] invocations = new Invocation[params.length];
> for (int i = 0; i < params.length; i++)
> invocations[i] = new Invocation(method, params[i]);
> CLIENT = (Client) conf.getObject(Client.class.getName());
> if(CLIENT == null) {
> CLIENT = new Client(ObjectWritable.class, conf);
> conf.setObject(Client.class.getName(), CLIENT);
> }
> Writable[] wrappedValues = CLIENT.call(invocations, addrs);
>
> if (method.getReturnType() == Void.TYPE) {
> return null;
> }
> Object[] values =
> (Object[])Array.newInstance(method.getReturnType(),wrappedValues.length);
> for (int i = 0; i < values.length; i++)
> if (wrappedValues[i] != null)
> values[i] = ((ObjectWritable)wrappedValues[i]).get();
>
> return values;
> }.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira