> On Nov. 6, 2014, 5:47 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/conf/Property.java, line 276
> > <https://reviews.apache.org/r/27654/diff/3/?file=751136#file751136line276>
> >
> >     I think this default may be too low for reasons you mentioned on irc 
> > (like a walog recovery taking a while).  I am thinking 5 or 10 min would be 
> > better.  We don't want people to ignore the warning, if it happens more 
> > than it should.

Yeah, I knew I needed to come back to this. I'm unsure what a good reasonable 
value is. I even considered 15mins.


> On Nov. 6, 2014, 5:47 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/ActiveAssignmentRunnable.java,
> >  line 28
> > <https://reviews.apache.org/r/27654/diff/3/?file=751137#file751137line28>
> >
> >     It seems LoggingRunnable's run method is never executed?

Good catch. Looking back at this, I don't think I want ActiveAssignmentRunnable 
to be a LoggingRunnable, but just accept a LoggingRunnable instead.


> On Nov. 6, 2014, 5:47 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java,
> >  line 203
> > <https://reviews.apache.org/r/27654/diff/3/?file=751140#file751140line203>
> >
> >     why have two maps?

Oh, true. I was just keeping metadata and normal assignments separate, but the 
keyspace will never overlap.


> On Nov. 6, 2014, 5:47 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java,
> >  line 248
> > <https://reviews.apache.org/r/27654/diff/3/?file=751140#file751140line248>
> >
> >     15 seconds?

Heh, thanks. I had the timeout really small before I added configuration to 
test things.


> On Nov. 6, 2014, 5:47 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java,
> >  line 250
> > <https://reviews.apache.org/r/27654/diff/3/?file=751140#file751140line250>
> >
> >     The compaction code remembers when it logged an exception and does not 
> > do it again.   It also logs a message if the compaction becomes unstuck.  
> > An advantage I thought of w/ repeatedly logging, is that you could see the 
> > stack trace changing (or not).
> >     
> >     
> >     The stack trace is  a possible trace.  By the time logging happens, the 
> > assignment could have completed and the thread could have moved on to other 
> > things.

Yeah, since these are running fairly regularly (order of seconds) a stuck 
assignment could get really spammy. Like you point out, there could be value 
gained from printing out the stack more than once. Maybe I could add some 
backoff which only warns so often?

bq. By the time logging happens, the assignment could have completed and the 
thread could have moved on to other things.

Do you think the message should be updated to be more clear about this? A 
"Maybe you should look into this" type message?


- Josh


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


On Nov. 6, 2014, 12:58 a.m., Josh Elser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27654/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2014, 12:58 a.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3304
>     https://issues.apache.org/jira/browse/ACCUMULO-3304
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Watches assignments and reports when an assignment is running for longer than 
> a configured time.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 56f3d9c 
>   
> server/tserver/src/main/java/org/apache/accumulo/tserver/ActiveAssignmentRunnable.java
>  PRE-CREATION 
>   
> server/tserver/src/main/java/org/apache/accumulo/tserver/RunnableStartedAt.java
>  PRE-CREATION 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 
> 94be0bb 
>   
> server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java
>  935ffeb 
> 
> Diff: https://reviews.apache.org/r/27654/diff/
> 
> 
> Testing
> -------
> 
> Very minimal.
> 
> 
> Thanks,
> 
> Josh Elser
> 
>

Reply via email to