[jira] [Commented] (HBASE-9987) Remove some synchronisation points in HConnectionManager
[ https://issues.apache.org/jira/browse/HBASE-9987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13852262#comment-13852262 ] stack commented on HBASE-9987: -- +1 on this change as is. +1 on your suggestions above (remove force delete and how to return location... and to removing all code under lock today) > Remove some synchronisation points in HConnectionManager > > > Key: HBASE-9987 > URL: https://issues.apache.org/jira/browse/HBASE-9987 > Project: HBase > Issue Type: Bug > Components: Client >Affects Versions: 0.98.0, 0.96.0 >Reporter: Nicolas Liochon >Assignee: Nicolas Liochon > Fix For: 0.98.0, 0.96.1 > > Attachments: 9987.v1.patch, 9987.v2.patch > > > Change a Map to a concurrentMap > Removed the "cachedServer (introduced in HBASE-4785). I suspect that this > function is not needed anymore as we also have a list of dead servers, and > accessing the list is not blocking. I will dig into this more however. > The patch gives a 10% improvement with the NoClusterClient. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HBASE-9987) Remove some synchronisation points in HConnectionManager
[ https://issues.apache.org/jira/browse/HBASE-9987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13850314#comment-13850314 ] Nicolas Liochon commented on HBASE-9987: bq. we just miss cache a few times more for a better perf overall? It's difficult to be sure. But let say that if someone just removed the entry from the cache, it makes sense to to recheck again (and to add the entry again to be cache)... I feel that the shorter the synchronisation time the better it is, but there is nothing proven. bq. There seems to be a bit of dodgy updating now we no longer have sync blocks. I think there are two things we should do: - removing the force delete: it's just bad imho: because someone does not want to use the cache, it removes the entry for everybody! As well, we're doing it inside the retry loop. That's strange. - when we use meta, instead of doing: ?? loc = call meta; cache(loc); return getCache()?? we should do ?? loc = call meta; cache(loc); return loc??. That's what we're doing outside of the lock actually. It's the prefect implementation that leads us to the logic. I'm waiting for the reverseScan before removing the prefect (this way we will do a single call instead of two as today). But may be I should remove the prefect immediately, keeping the two calls. The simplest implementation would just be removing all the code that is today in the lock. > Remove some synchronisation points in HConnectionManager > > > Key: HBASE-9987 > URL: https://issues.apache.org/jira/browse/HBASE-9987 > Project: HBase > Issue Type: Bug > Components: Client >Affects Versions: 0.98.0, 0.96.0 >Reporter: Nicolas Liochon >Assignee: Nicolas Liochon > Fix For: 0.98.0, 0.96.1 > > Attachments: 9987.v1.patch, 9987.v2.patch > > > Change a Map to a concurrentMap > Removed the "cachedServer (introduced in HBASE-4785). I suspect that this > function is not needed anymore as we also have a list of dead servers, and > accessing the list is not blocking. I will dig into this more however. > The patch gives a 10% improvement with the NoClusterClient. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HBASE-9987) Remove some synchronisation points in HConnectionManager
[
https://issues.apache.org/jira/browse/HBASE-9987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13849939#comment-13849939
]
stack commented on HBASE-9987:
--
Is this ok?
+ Map tableLocations =
getTableLocations(tableName);
+ // start to examine the cache. we can only do cache actions
+ // if there's something in the cache for this table.
+ rl = getCachedLocation(tableName, row);
+ if (rl != null) {
+tableLocations.remove(rl.getRegionInfo().getStartKey());
}
Could another thread have set into tableLocations a good location and we are
removing it here? Previous it couldn't have happened because there was the
sync block around the whole operation.
Ditto here:
+ result = this.cachedRegionLocations.get(tableName);
+ // if tableLocations for this table isn't built yet, make one
+ if (result == null) {
+result = new ConcurrentSkipListMap(Bytes.BYTES_COMPARATOR);
+ConcurrentSkipListMap old =
+this.cachedRegionLocations.putIfAbsent(tableName, result);
+if (old != null) {
+ return old;
Could another thread have inserted a result with perhaps some values in it?
I suppose it is not the end of the world if this happens We'll just miss
the cache a bit more often.
Could this be a problem?
- synchronized(this.cachedRegionLocations) {
-this.cachedRegionLocations.clear();
-this.cachedServers.clear();
- }
+ this.cachedRegionLocations.clear();
+ this.cachedServers.clear();
Could we give a bad location out if cachedRegionLocations is cleared but
cachedServers is not?
There seems to be a bit of dodgy updating now we no longer have sync blocks.
but perhaps it is ok; we just miss cache a few times more for a better perf
overall?
> Remove some synchronisation points in HConnectionManager
>
>
> Key: HBASE-9987
> URL: https://issues.apache.org/jira/browse/HBASE-9987
> Project: HBase
> Issue Type: Bug
> Components: Client
>Affects Versions: 0.98.0, 0.96.0
>Reporter: Nicolas Liochon
>Assignee: Nicolas Liochon
> Fix For: 0.98.0, 0.96.1
>
> Attachments: 9987.v1.patch, 9987.v2.patch
>
>
> Change a Map to a concurrentMap
> Removed the "cachedServer (introduced in HBASE-4785). I suspect that this
> function is not needed anymore as we also have a list of dead servers, and
> accessing the list is not blocking. I will dig into this more however.
> The patch gives a 10% improvement with the NoClusterClient.
--
This message was sent by Atlassian JIRA
(v6.1.4#6159)
[jira] [Commented] (HBASE-9987) Remove some synchronisation points in HConnectionManager
[ https://issues.apache.org/jira/browse/HBASE-9987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13849936#comment-13849936 ] stack commented on HBASE-9987: -- +1 > Remove some synchronisation points in HConnectionManager > > > Key: HBASE-9987 > URL: https://issues.apache.org/jira/browse/HBASE-9987 > Project: HBase > Issue Type: Bug > Components: Client >Affects Versions: 0.98.0, 0.96.0 >Reporter: Nicolas Liochon >Assignee: Nicolas Liochon > Fix For: 0.98.0, 0.96.1 > > Attachments: 9987.v1.patch, 9987.v2.patch > > > Change a Map to a concurrentMap > Removed the "cachedServer (introduced in HBASE-4785). I suspect that this > function is not needed anymore as we also have a list of dead servers, and > accessing the list is not blocking. I will dig into this more however. > The patch gives a 10% improvement with the NoClusterClient. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HBASE-9987) Remove some synchronisation points in HConnectionManager
[ https://issues.apache.org/jira/browse/HBASE-9987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13826320#comment-13826320 ] Hudson commented on HBASE-9987: --- SUCCESS: Integrated in HBase-TRUNK #4687 (See [https://builds.apache.org/job/HBase-TRUNK/4687/]) HBASE-9987 Remove some synchronisation points in HConnectionManager (nkeywal: rev 1543051) * /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java > Remove some synchronisation points in HConnectionManager > > > Key: HBASE-9987 > URL: https://issues.apache.org/jira/browse/HBASE-9987 > Project: HBase > Issue Type: Bug > Components: Client >Affects Versions: 0.98.0, 0.96.0 >Reporter: Nicolas Liochon >Assignee: Nicolas Liochon > Fix For: 0.98.0, 0.96.1 > > Attachments: 9987.v1.patch, 9987.v2.patch > > > Change a Map to a concurrentMap > Removed the "cachedServer (introduced in HBASE-4785). I suspect that this > function is not needed anymore as we also have a list of dead servers, and > accessing the list is not blocking. I will dig into this more however. > The patch gives a 10% improvement with the NoClusterClient. -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (HBASE-9987) Remove some synchronisation points in HConnectionManager
[ https://issues.apache.org/jira/browse/HBASE-9987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13826267#comment-13826267 ] Hudson commented on HBASE-9987: --- SUCCESS: Integrated in hbase-0.96 #195 (See [https://builds.apache.org/job/hbase-0.96/195/]) HBASE-9987 Remove some synchronisation points in HConnectionManager (nkeywal: rev 1543050) * /hbase/branches/0.96/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java > Remove some synchronisation points in HConnectionManager > > > Key: HBASE-9987 > URL: https://issues.apache.org/jira/browse/HBASE-9987 > Project: HBase > Issue Type: Bug > Components: Client >Affects Versions: 0.98.0, 0.96.0 >Reporter: Nicolas Liochon >Assignee: Nicolas Liochon > Fix For: 0.98.0, 0.96.1 > > Attachments: 9987.v1.patch, 9987.v2.patch > > > Change a Map to a concurrentMap > Removed the "cachedServer (introduced in HBASE-4785). I suspect that this > function is not needed anymore as we also have a list of dead servers, and > accessing the list is not blocking. I will dig into this more however. > The patch gives a 10% improvement with the NoClusterClient. -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (HBASE-9987) Remove some synchronisation points in HConnectionManager
[ https://issues.apache.org/jira/browse/HBASE-9987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13825600#comment-13825600 ] Hudson commented on HBASE-9987: --- SUCCESS: Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #842 (See [https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/842/]) HBASE-9987 Remove some synchronisation points in HConnectionManager (nkeywal: rev 1543051) * /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java > Remove some synchronisation points in HConnectionManager > > > Key: HBASE-9987 > URL: https://issues.apache.org/jira/browse/HBASE-9987 > Project: HBase > Issue Type: Bug > Components: Client >Affects Versions: 0.98.0, 0.96.0 >Reporter: Nicolas Liochon >Assignee: Nicolas Liochon > Fix For: 0.98.0, 0.96.1 > > Attachments: 9987.v1.patch, 9987.v2.patch > > > Change a Map to a concurrentMap > Removed the "cachedServer (introduced in HBASE-4785). I suspect that this > function is not needed anymore as we also have a list of dead servers, and > accessing the list is not blocking. I will dig into this more however. > The patch gives a 10% improvement with the NoClusterClient. -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (HBASE-9987) Remove some synchronisation points in HConnectionManager
[ https://issues.apache.org/jira/browse/HBASE-9987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13825546#comment-13825546 ] Hudson commented on HBASE-9987: --- SUCCESS: Integrated in hbase-0.96-hadoop2 #123 (See [https://builds.apache.org/job/hbase-0.96-hadoop2/123/]) HBASE-9987 Remove some synchronisation points in HConnectionManager (nkeywal: rev 1543050) * /hbase/branches/0.96/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java > Remove some synchronisation points in HConnectionManager > > > Key: HBASE-9987 > URL: https://issues.apache.org/jira/browse/HBASE-9987 > Project: HBase > Issue Type: Bug > Components: Client >Affects Versions: 0.98.0, 0.96.0 >Reporter: Nicolas Liochon >Assignee: Nicolas Liochon > Fix For: 0.98.0, 0.96.1 > > Attachments: 9987.v1.patch, 9987.v2.patch > > > Change a Map to a concurrentMap > Removed the "cachedServer (introduced in HBASE-4785). I suspect that this > function is not needed anymore as we also have a list of dead servers, and > accessing the list is not blocking. I will dig into this more however. > The patch gives a 10% improvement with the NoClusterClient. -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (HBASE-9987) Remove some synchronisation points in HConnectionManager
[
https://issues.apache.org/jira/browse/HBASE-9987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13825354#comment-13825354
]
Hadoop QA commented on HBASE-9987:
--
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12614364/9987.v2.patch
against trunk revision .
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:red}-1 tests included{color}. The patch doesn't appear to include
any new or modified tests.
Please justify why no new tests are needed for this
patch.
Also please list what manual steps were performed to
verify this patch.
{color:green}+1 hadoop1.0{color}. The patch compiles against the hadoop
1.0 profile.
{color:green}+1 hadoop2.0{color}. The patch compiles against the hadoop
2.0 profile.
{color:red}-1 javadoc{color}. The javadoc tool appears to have generated
10 warning messages.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 1.3.9) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 lineLengths{color}. The patch does not introduce lines
longer than 100
{color:red}-1 site{color}. The patch appears to cause mvn site goal to
fail.
{color:green}+1 core tests{color}. The patch passed unit tests in .
Test results:
https://builds.apache.org/job/PreCommit-HBASE-Build/7913//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7913//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7913//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7913//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7913//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7913//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7913//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7913//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7913//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7913//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output:
https://builds.apache.org/job/PreCommit-HBASE-Build/7913//console
This message is automatically generated.
> Remove some synchronisation points in HConnectionManager
>
>
> Key: HBASE-9987
> URL: https://issues.apache.org/jira/browse/HBASE-9987
> Project: HBase
> Issue Type: Bug
> Components: Client
>Affects Versions: 0.98.0, 0.96.0
>Reporter: Nicolas Liochon
>Assignee: Nicolas Liochon
> Fix For: 0.98.0, 0.96.1
>
> Attachments: 9987.v1.patch, 9987.v2.patch
>
>
> Change a Map to a concurrentMap
> Removed the "cachedServer (introduced in HBASE-4785). I suspect that this
> function is not needed anymore as we also have a list of dead servers, and
> accessing the list is not blocking. I will dig into this more however.
> The patch gives a 10% improvement with the NoClusterClient.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9987) Remove some synchronisation points in HConnectionManager
[ https://issues.apache.org/jira/browse/HBASE-9987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13825238#comment-13825238 ] Nicolas Liochon commented on HBASE-9987: In v2 I've put back the cachedServer. We could do better than this, but short term it's safer to keep it. That's the only difference, so I will commit if the unit tests pass. > Remove some synchronisation points in HConnectionManager > > > Key: HBASE-9987 > URL: https://issues.apache.org/jira/browse/HBASE-9987 > Project: HBase > Issue Type: Bug > Components: Client >Affects Versions: 0.98.0, 0.96.0 >Reporter: Nicolas Liochon >Assignee: Nicolas Liochon > Fix For: 0.98.0, 0.96.1 > > Attachments: 9987.v1.patch, 9987.v2.patch > > > Change a Map to a concurrentMap > Removed the "cachedServer (introduced in HBASE-4785). I suspect that this > function is not needed anymore as we also have a list of dead servers, and > accessing the list is not blocking. I will dig into this more however. > The patch gives a 10% improvement with the NoClusterClient. -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (HBASE-9987) Remove some synchronisation points in HConnectionManager
[ https://issues.apache.org/jira/browse/HBASE-9987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13824967#comment-13824967 ] Lars Hofhansl commented on HBASE-9987: -- +1 Will add the ConcurrentMap stuff to 0.94 as well. > Remove some synchronisation points in HConnectionManager > > > Key: HBASE-9987 > URL: https://issues.apache.org/jira/browse/HBASE-9987 > Project: HBase > Issue Type: Bug > Components: Client >Affects Versions: 0.98.0, 0.96.0 >Reporter: Nicolas Liochon >Assignee: Nicolas Liochon > Fix For: 0.98.0, 0.96.1 > > Attachments: 9987.v1.patch > > > Change a Map to a concurrentMap > Removed the "cachedServer (introduced in HBASE-4785). I suspect that this > function is not needed anymore as we also have a list of dead servers, and > accessing the list is not blocking. I will dig into this more however. > The patch gives a 10% improvement with the NoClusterClient. -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (HBASE-9987) Remove some synchronisation points in HConnectionManager
[ https://issues.apache.org/jira/browse/HBASE-9987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13824886#comment-13824886 ] Ted Yu commented on HBASE-9987: --- +1 > Remove some synchronisation points in HConnectionManager > > > Key: HBASE-9987 > URL: https://issues.apache.org/jira/browse/HBASE-9987 > Project: HBase > Issue Type: Bug > Components: Client >Affects Versions: 0.98.0, 0.96.0 >Reporter: Nicolas Liochon >Assignee: Nicolas Liochon > Fix For: 0.98.0, 0.96.1 > > Attachments: 9987.v1.patch > > > Change a Map to a concurrentMap > Removed the "cachedServer (introduced in HBASE-4785). I suspect that this > function is not needed anymore as we also have a list of dead servers, and > accessing the list is not blocking. I will dig into this more however. > The patch gives a 10% improvement with the NoClusterClient. -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (HBASE-9987) Remove some synchronisation points in HConnectionManager
[
https://issues.apache.org/jira/browse/HBASE-9987?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13824860#comment-13824860
]
Hadoop QA commented on HBASE-9987:
--
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12614282/9987.v1.patch
against trunk revision .
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 3 new
or modified tests.
{color:green}+1 hadoop1.0{color}. The patch compiles against the hadoop
1.0 profile.
{color:green}+1 hadoop2.0{color}. The patch compiles against the hadoop
2.0 profile.
{color:red}-1 javadoc{color}. The javadoc tool appears to have generated
10 warning messages.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 1.3.9) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 lineLengths{color}. The patch does not introduce lines
longer than 100
{color:red}-1 site{color}. The patch appears to cause mvn site goal to
fail.
{color:green}+1 core tests{color}. The patch passed unit tests in .
Test results:
https://builds.apache.org/job/PreCommit-HBASE-Build/7903//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7903//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7903//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7903//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7903//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7903//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7903//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7903//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7903//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/7903//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output:
https://builds.apache.org/job/PreCommit-HBASE-Build/7903//console
This message is automatically generated.
> Remove some synchronisation points in HConnectionManager
>
>
> Key: HBASE-9987
> URL: https://issues.apache.org/jira/browse/HBASE-9987
> Project: HBase
> Issue Type: Bug
> Components: Client
>Affects Versions: 0.98.0, 0.96.0
>Reporter: Nicolas Liochon
>Assignee: Nicolas Liochon
> Fix For: 0.98.0, 0.96.1
>
> Attachments: 9987.v1.patch
>
>
> Change a Map to a concurrentMap
> Removed the "cachedServer (introduced in HBASE-4785). I suspect that this
> function is not needed anymore as we also have a list of dead servers, and
> accessing the list is not blocking. I will dig into this more however.
> The patch gives a 10% improvement with the NoClusterClient.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
