Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Flume >> mail # dev >> Review Request: FLUME-1896. Thrift Rpc Client


Copy link to this message
-
Re: Review Request: FLUME-1896. Thrift Rpc Client.


> On Feb. 10, 2013, 9:11 p.m., Brock Noland wrote:
> > flume-ng-sdk/src/main/java/org/apache/flume/api/ThriftRpcClient.java, line 337
> > <https://reviews.apache.org/r/9284/diff/10/?file=257203#file257203line337>
> >
> >     Would a small amount of synchronization and a BlockingQueue make this code a little simpler?
>
> Hari Shreedharan wrote:
>     I don't see it becoming much simpler. I also prefer having the threading code being more explicit. A blocking queue does not really help, since we are lazily initializing the clients, so we need locking for that anyway.

Sorry I thought I had replaced this comment with the one below.
> On Feb. 10, 2013, 9:11 p.m., Brock Noland wrote:
> > flume-ng-sdk/src/main/java/org/apache/flume/api/ThriftRpcClient.java, line 337
> > <https://reviews.apache.org/r/9284/diff/10/?file=257203#file257203line337>
> >
> >     I feel like we could do this in a little simpler fashion with a Semaphore?

Hari,

Right now the checkout method has 9 lines of code doing non initialization work. Could this be done in 4-5 lines with a semaphore? I feel that is simpler and achieves the same result.

semaphore.acquire(1);
o = availableClients.poll();
if(o == null) {
  o = createNew();
  checkedOutClients.add(o);
}

then on return

availableClients.put(o);
checkedOutClients.remove(o);
semaphore.release(1);
- Brock
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9284/#review16399
-----------------------------------------------------------
On Feb. 9, 2013, 2:28 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9284/
> -----------------------------------------------------------
>
> (Updated Feb. 9, 2013, 2:28 a.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Added thrift rpc client. Detailed description posted on jira.
>
>
> This addresses bug FLUME-1896.
>     https://issues.apache.org/jira/browse/FLUME-1896
>
>
> Diffs
> -----
>
>   flume-ng-legacy-sources/flume-thrift-source/pom.xml b9667cd
>   flume-ng-legacy-sources/flume-thrift-source/src/main/java/com/cloudera/flume/handlers/thrift/EventStatus.java 327107a
>   flume-ng-legacy-sources/flume-thrift-source/src/main/java/com/cloudera/flume/handlers/thrift/Priority.java d2495d2
>   flume-ng-legacy-sources/flume-thrift-source/src/main/java/com/cloudera/flume/handlers/thrift/ThriftFlumeEvent.java 2bb6cfd
>   flume-ng-legacy-sources/flume-thrift-source/src/main/java/com/cloudera/flume/handlers/thrift/ThriftFlumeEventServer.java 0f2ad2d
>   flume-ng-legacy-sources/flume-thrift-source/src/main/thrift/aslv2 PRE-CREATION
>   flume-ng-sdk/pom.xml ab066d5
>   flume-ng-sdk/src/main/java/org/apache/flume/api/HostInfo.java 7388a45
>   flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java ab4c3de
>   flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 3c93921
>   flume-ng-sdk/src/main/java/org/apache/flume/api/ThriftRpcClient.java PRE-CREATION
>   flume-ng-sdk/src/main/java/org/apache/flume/thrift/Status.java PRE-CREATION
>   flume-ng-sdk/src/main/java/org/apache/flume/thrift/ThriftFlumeEvent.java PRE-CREATION
>   flume-ng-sdk/src/main/java/org/apache/flume/thrift/ThriftSourceProtocol.java PRE-CREATION
>   flume-ng-sdk/src/main/thrift/aslv2 PRE-CREATION
>   flume-ng-sdk/src/main/thrift/flume.thrift PRE-CREATION
>   flume-ng-sdk/src/test/java/org/apache/flume/api/TestThriftRpcClient.java PRE-CREATION
>   flume-ng-sdk/src/test/java/org/apache/flume/api/ThriftTestingSource.java PRE-CREATION
>   flume-ng-sources/flume-scribe-source/pom.xml 588207d
>   flume-ng-sources/flume-scribe-source/src/main/java/org/apache/flume/source/scribe/LogEntry.java 401a7e4
>   flume-ng-sources/flume-scribe-source/src/main/java/org/apache/flume/source/scribe/ResultCode.java 6bfa84c