accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Josh Elser <josh.el...@gmail.com>
Subject Re: Review Request 43957: ACCUMULO-1755: removed synchronized modifier from TabletServerBatchWriterstartProcessing()
Date Wed, 24 Feb 2016 19:15:59 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43957/#review120541
-----------------------------------------------------------




core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java (line
116)
<https://reviews.apache.org/r/43957/#comment182004>

    Can make this `final` now



core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java (line
223)
<https://reviews.apache.org/r/43957/#comment182009>

    Can be `final`.



core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java (line
227)
<https://reviews.apache.org/r/43957/#comment182005>

    Semantically, this doesn't look any different, but it would be a little clearer if you
didn't invert the order of these two calls.



core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java (line
967)
<https://reviews.apache.org/r/43957/#comment182007>

    Can be `final` now. Also, why the upgrade to Long?



core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java (line
974)
<https://reviews.apache.org/r/43957/#comment182006>

    Don't need to specify the parameterization of the HashMap


I'm trying to look at the big picture here as well, but I'm not sure that this change actually
helps the synchronization. It seems like every call to `startProcessing` is made by a `synchronized`
method (flush, addMutation, close, addFailedMutations) or while holding `TabletServerBatchWriter.class`'s
lock. Am I missing something?

- Josh Elser


On Feb. 24, 2016, 6:54 p.m., Dave Marion wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43957/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2016, 6:54 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> ACCUMULO-1755: removed synchronized modifier from TabletServerBatchWriterstartProcessing()
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java
bc90d00 
> 
> Diff: https://reviews.apache.org/r/43957/diff/
> 
> 
> Testing
> -------
> 
> unit tests in core pass
> 
> 
> Thanks,
> 
> Dave Marion
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message