[
https://issues.apache.org/jira/browse/HBASE-8706?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13678108#comment-13678108
]
Matteo Bertozzi commented on HBASE-8706:
----------------------------------------
* make "timeoutMillis" final
* follow the same style... there's a this.xyz for the other initializations
* the Procedure stuff is generic... and doesn't talk about snapshots... so
avoid the SNAPSHOT_TIMEOUT constant inside the procedure code. Instead of
passing a conf pass a timeout and let the snapshot manager deal with the conf?
since you're cleaning up, do you mind removing also this unuseful log message?
{code}
+++ hbase/regionserver/snapshot/RegionServerSnapshotManager.java
// evict remaining tasks and futures from taskPool.
- LOG.debug(taskPool);
while (!futures.isEmpty()) {
// block to remove cancelled futures;
LOG.warn("Removing cancelled elements from taskPool");
{code}
also do you mind cleanup this related thing?
* The "hbase.snapshot.master.timeoutMillis" seems to be used with a different
meaning... if I read correctly is not a snapshot timeout but a thread pool
cleanup on idle. (rename it?)
> Some improvement in snapshot
> ----------------------------
>
> Key: HBASE-8706
> URL: https://issues.apache.org/jira/browse/HBASE-8706
> Project: HBase
> Issue Type: Bug
> Components: snapshots
> Affects Versions: 0.94.8, 0.95.0
> Reporter: binlijin
> Attachments: HBASE-8706.patch
>
>
> (1)timeout for Procedure can not be configured.
> {code}
> Procedure's timeout
> ProcedureCoordinator
> final static long TIMEOUT_MILLIS_DEFAULT = 60000;
> createProcedure(ForeignExceptionDispatcher fed, String procName, byte[]
> procArgs,
> List<String> expectedMembers) {
> // build the procedure
> return new Procedure(this, fed, WAKE_MILLIS_DEFAULT,
> TIMEOUT_MILLIS_DEFAULT,
> procName, procArgs, expectedMembers);
> }
> RegionServerSnapshotManager:
> /** Conf key for max time to keep threads in snapshot request pool waiting
> */
> public static final String SNAPSHOT_TIMEOUT_MILLIS_KEY =
> "hbase.snapshot.region.timeout";
> /** Keep threads alive in request pool for max of 60 seconds */
> public static final long SNAPSHOT_TIMEOUT_MILLIS_DEFAULT = 60000;
> public Subprocedure buildSubprocedure(SnapshotDescription snapshot) {
> long timeoutMillis = conf.getLong(SNAPSHOT_TIMEOUT_MILLIS_KEY,
> SNAPSHOT_TIMEOUT_MILLIS_DEFAULT);
> case FLUSH:
> SnapshotSubprocedurePool taskManager =
> new SnapshotSubprocedurePool(rss.getServerName().toString(), conf);
> }
> {code}
> (2)TakeSnapshotHandler
> after snapshotRegions we should call monitor.rethrowException(); to check if
> there is exception and if there is we can skip the verifySnapshot
> (3)too much error message when error happened in some place.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
