[jira] [Commented] (HBASE-16668) Admin class should have a synchronous Admin#mergeRegions* method
[
https://issues.apache.org/jira/browse/HBASE-16668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16462226#comment-16462226
]
Guanghao Zhang commented on HBASE-16668:
Seems we already have a mergeRegions method and marked deprecated in
HBASE-14552... The reason is?
> Admin class should have a synchronous Admin#mergeRegions* method
> -
>
> Key: HBASE-16668
> URL: https://issues.apache.org/jira/browse/HBASE-16668
> Project: HBase
> Issue Type: Bug
> Components: hbase
>Affects Versions: 2.0.0
>Reporter: Jonathan Hsieh
>Assignee: Stephen Yuan Jiang
>Priority: Major
> Fix For: 3.0.0
>
> Attachments: 16668.v1.txt
>
>
> In trunk from HBASE-14552, we have deprecated {{void Admin#mergeRegions}} (in
> 1.x this was an asynchronous call) and replaced it with {{Future
> Admin#mergeRegionsAsync}} which is clearly async.
> This leaves us only with the async version.
> We should have an easy way to make {{mergeRegions}} or an equivalant behave
> synchronously.
> For normal java Futures, we could just call the future's {{get()}} method.
> Unforutnately, the future this method returns doesn't follow java Future
> convention and throws Unimplemented operation when a plain {{get()}} is
> called and makes the api harder to use and read. We could make this future
> act more normally, and have the timeout throw an InterruptedException.
> Alternately, we could expose a new method in {{Admin}} that behaves
> synchronously such as {{HBaseAdmin#mergeRegionsSync}}. The caveat here is
> that we shouldn't use the name {{#mergeRegions}} since it exists in 1.x with
> async semantics.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-16668) Admin class should have a synchronous Admin#mergeRegions* method
[
https://issues.apache.org/jira/browse/HBASE-16668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=1646#comment-1646
]
Guanghao Zhang commented on HBASE-16668:
Don't need sync as the method end. We should use the same pattern for methods
in Admin. etc. modifyTable for sync method and modifyTableAsync for async
method. Thanks.
> Admin class should have a synchronous Admin#mergeRegions* method
> -
>
> Key: HBASE-16668
> URL: https://issues.apache.org/jira/browse/HBASE-16668
> Project: HBase
> Issue Type: Bug
> Components: hbase
>Affects Versions: 2.0.0
>Reporter: Jonathan Hsieh
>Assignee: Stephen Yuan Jiang
>Priority: Major
> Fix For: 3.0.0
>
> Attachments: 16668.v1.txt
>
>
> In trunk from HBASE-14552, we have deprecated {{void Admin#mergeRegions}} (in
> 1.x this was an asynchronous call) and replaced it with {{Future
> Admin#mergeRegionsAsync}} which is clearly async.
> This leaves us only with the async version.
> We should have an easy way to make {{mergeRegions}} or an equivalant behave
> synchronously.
> For normal java Futures, we could just call the future's {{get()}} method.
> Unforutnately, the future this method returns doesn't follow java Future
> convention and throws Unimplemented operation when a plain {{get()}} is
> called and makes the api harder to use and read. We could make this future
> act more normally, and have the timeout throw an InterruptedException.
> Alternately, we could expose a new method in {{Admin}} that behaves
> synchronously such as {{HBaseAdmin#mergeRegionsSync}}. The caveat here is
> that we shouldn't use the name {{#mergeRegions}} since it exists in 1.x with
> async semantics.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-16668) Admin class should have a synchronous Admin#mergeRegions* method
[
https://issues.apache.org/jira/browse/HBASE-16668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16462188#comment-16462188
]
Ted Yu commented on HBASE-16668:
[~zghaobac]
See the above question.
> Admin class should have a synchronous Admin#mergeRegions* method
> -
>
> Key: HBASE-16668
> URL: https://issues.apache.org/jira/browse/HBASE-16668
> Project: HBase
> Issue Type: Bug
> Components: hbase
>Affects Versions: 2.0.0
>Reporter: Jonathan Hsieh
>Assignee: Stephen Yuan Jiang
>Priority: Major
> Fix For: 3.0.0
>
> Attachments: 16668.v1.txt
>
>
> In trunk from HBASE-14552, we have deprecated {{void Admin#mergeRegions}} (in
> 1.x this was an asynchronous call) and replaced it with {{Future
> Admin#mergeRegionsAsync}} which is clearly async.
> This leaves us only with the async version.
> We should have an easy way to make {{mergeRegions}} or an equivalant behave
> synchronously.
> For normal java Futures, we could just call the future's {{get()}} method.
> Unforutnately, the future this method returns doesn't follow java Future
> convention and throws Unimplemented operation when a plain {{get()}} is
> called and makes the api harder to use and read. We could make this future
> act more normally, and have the timeout throw an InterruptedException.
> Alternately, we could expose a new method in {{Admin}} that behaves
> synchronously such as {{HBaseAdmin#mergeRegionsSync}}. The caveat here is
> that we shouldn't use the name {{#mergeRegions}} since it exists in 1.x with
> async semantics.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-16668) Admin class should have a synchronous Admin#mergeRegions* method
[
https://issues.apache.org/jira/browse/HBASE-16668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16458991#comment-16458991
]
Ted Yu commented on HBASE-16668:
{code}
[ERROR] Run 1:
TestInterfaceAlign.testAdminWithAsyncAdmin:62->lambda$testAdminWithAsyncAdmin$0:67
Admin method mergeRegionsSync should in AsyncAdmin too
{code}
[~zghaobac]:
Do you have suggestion on how the above should be addressed ?
I wonder if mergeRegionsSync should return CompletableFuture.
> Admin class should have a synchronous Admin#mergeRegions* method
> -
>
> Key: HBASE-16668
> URL: https://issues.apache.org/jira/browse/HBASE-16668
> Project: HBase
> Issue Type: Bug
> Components: hbase
>Affects Versions: 2.0.0
>Reporter: Jonathan Hsieh
>Assignee: Stephen Yuan Jiang
>Priority: Major
> Fix For: 3.0.0
>
> Attachments: 16668.v1.txt
>
>
> In trunk from HBASE-14552, we have deprecated {{void Admin#mergeRegions}} (in
> 1.x this was an asynchronous call) and replaced it with {{Future
> Admin#mergeRegionsAsync}} which is clearly async.
> This leaves us only with the async version.
> We should have an easy way to make {{mergeRegions}} or an equivalant behave
> synchronously.
> For normal java Futures, we could just call the future's {{get()}} method.
> Unforutnately, the future this method returns doesn't follow java Future
> convention and throws Unimplemented operation when a plain {{get()}} is
> called and makes the api harder to use and read. We could make this future
> act more normally, and have the timeout throw an InterruptedException.
> Alternately, we could expose a new method in {{Admin}} that behaves
> synchronously such as {{HBaseAdmin#mergeRegionsSync}}. The caveat here is
> that we shouldn't use the name {{#mergeRegions}} since it exists in 1.x with
> async semantics.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-16668) Admin class should have a synchronous Admin#mergeRegions* method
[
https://issues.apache.org/jira/browse/HBASE-16668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16458900#comment-16458900
]
Hadoop QA commented on HBASE-16668:
---
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
34s{color} | {color:blue} Docker mode activated. {color} |
| {color:blue}0{color} | {color:blue} patch {color} | {color:blue} 0m
1s{color} | {color:blue} The patch file was not named according to hbase's
naming conventions. Please see
https://yetus.apache.org/documentation/0.7.0/precommit-patchnames for
instructions. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green} 0m
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} 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} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 4m
44s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
40s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
33s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 5m
0s{color} | {color:green} branch has no errors when building our shaded
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
1s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
22s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 4m
56s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
36s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
36s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m
35s{color} | {color:red} hbase-client: The patch generated 1 new + 170
unchanged - 0 fixed = 171 total (was 170) {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 4m
55s{color} | {color:green} patch has no errors when building our shaded
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}
14m 39s{color} | {color:green} Patch does not cause any errors with Hadoop
2.6.5 2.7.4 or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
5s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
21s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 1m 43s{color}
| {color:red} hbase-client in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
9s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 42m 17s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | hadoop.hbase.client.TestInterfaceAlign |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:d8b550f |
| JIRA Issue | HBASE-16668 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12921281/16668.v1.txt |
| Optional Tests | asflicense javac javadoc unit findbugs shadedjars
hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 7b89088562e1 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9
14:43:09 UTC 2018 x86_64 GNU/Linux |
| Build tool | maven |
| Personality |
/home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh
|
| git revision | master / 6d080762ef |
| maven | version: Apache Maven 3.5.3
[jira] [Commented] (HBASE-16668) Admin class should have a synchronous Admin#mergeRegions* method
[
https://issues.apache.org/jira/browse/HBASE-16668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15707106#comment-15707106
]
Stephen Yuan Jiang commented on HBASE-16668:
As a side effect :-) of HBASE-14552, I created a synchronous version of merge
region - {{HBaseAdmin#mergeRegionsSync() }} - for testing-only in HBaseAdmin
class
{code}
/**
* Merge two regions. Synchronous operation.
* Note: It is not feasible to predict the length of merge.
* Therefore, this is for internal testing only.
* @param nameOfRegionA encoded or full name of region a
* @param nameOfRegionB encoded or full name of region b
* @param forcible true if do a compulsory merge, otherwise we will only merge
* two adjacent regions
* @throws IOException
*/
@VisibleForTesting
public void mergeRegionsSync(
final byte[] nameOfRegionA,
final byte[] nameOfRegionB,
final boolean forcible) throws IOException {
get(
mergeRegionsAsync(nameOfRegionA, nameOfRegionB, forcible),
syncWaitTimeout,
TimeUnit.MILLISECONDS);
}
{code}
> Admin class should have a synchronous Admin#mergeRegions* method
> -
>
> Key: HBASE-16668
> URL: https://issues.apache.org/jira/browse/HBASE-16668
> Project: HBase
> Issue Type: Bug
> Components: hbase
>Affects Versions: 2.0.0
>Reporter: Jonathan Hsieh
> Fix For: 2.0.0
>
>
> In trunk from HBASE-14552, we have deprecated {{void Admin#mergeRegions}} (in
> 1.x this was an asynchronous call) and replaced it with {{Future
> Admin#mergeRegionsAsync}} which is clearly async.
> This leaves us only with the async version.
> We should have an easy way to make {{mergeRegions}} or an equivalant behave
> synchronously.
> For normal java Futures, we could just call the future's {{get()}} method.
> Unforutnately, the future this method returns doesn't follow java Future
> convention and throws Unimplemented operation when a plain {{get()}} is
> called and makes the api harder to use and read. We could make this future
> act more normally, and have the timeout throw an InterruptedException.
> Alternately, we could expose a new method in {{Admin}} that behaves
> synchronously such as {{HBaseAdmin#mergeRegionsSync}}. The caveat here is
> that we shouldn't use the name {{#mergeRegions}} since it exists in 1.x with
> async semantics.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-16668) Admin class should have a synchronous Admin#mergeRegions* method
[
https://issues.apache.org/jira/browse/HBASE-16668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15510801#comment-15510801
]
Stephen Yuan Jiang commented on HBASE-16668:
My take is that we can live with async (already live with async in the past)
and I don't see much benefit to have a sync version (also the trend in client
moves towards async). Unless there is a demand (and it would be really easy to
add the sync method, figuring out the name everyone accepts probably would
take longer than coding :-)).
> Admin class should have a synchronous Admin#mergeRegions* method
> -
>
> Key: HBASE-16668
> URL: https://issues.apache.org/jira/browse/HBASE-16668
> Project: HBase
> Issue Type: Bug
> Components: hbase
>Affects Versions: 2.0.0
>Reporter: Jonathan Hsieh
> Fix For: 2.0.0
>
>
> In trunk from HBASE-14552, we have deprecated {{void Admin#mergeRegions}} (in
> 1.x this was an asynchronous call) and replaced it with {{Future
> Admin#mergeRegionsAsync}} which is clearly async.
> This leaves us only with the async version.
> We should have an easy way to make {{mergeRegions}} or an equivalant behave
> synchronously.
> For normal java Futures, we could just call the future's {{get()}} method.
> Unforutnately, the future this method returns doesn't follow java Future
> convention and throws Unimplemented operation when a plain {{get()}} is
> called and makes the api harder to use and read. We could make this future
> act more normally, and have the timeout throw an InterruptedException.
> Alternately, we could expose a new method in {{Admin}} that behaves
> synchronously such as {{HBaseAdmin#mergeRegionsSync}}. The caveat here is
> that we shouldn't use the name {{#mergeRegions}} since it exists in 1.x with
> async semantics.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-16668) Admin class should have a synchronous Admin#mergeRegions* method
[
https://issues.apache.org/jira/browse/HBASE-16668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15510774#comment-15510774
]
Jonathan Hsieh commented on HBASE-16668:
[~mbertozzi], [~syuanjiang], this was introduced in HBASE-14552. What are your
opinions on this?
> Admin class should have a synchronous Admin#mergeRegions* method
> -
>
> Key: HBASE-16668
> URL: https://issues.apache.org/jira/browse/HBASE-16668
> Project: HBase
> Issue Type: Bug
> Components: hbase
>Affects Versions: 2.0.0
>Reporter: Jonathan Hsieh
> Fix For: 2.0.0
>
>
> In trunk from HBASE-14552, we have deprecated {{void Admin#mergeRegions}} (in
> 1.x this was an asynchronous call) and replaced it with {{Future
> Admin#mergeRegionsAsync}} which is clearly async.
> This leaves us only with the async version.
> We should have an easy way to make {{mergeRegions}} or an equivalant behave
> synchronously.
> For normal java Futures, we could just call the future's {{get()}} method.
> Unforutnately, the future this method returns doesn't follow java Future
> convention and throws Unimplemented operation when a plain {{get()}} is
> called and makes the api harder to use and read. We could make this future
> act more normally, and have the timeout throw an InterruptedException.
> Alternately, we could expose a new method in {{Admin}} that behaves
> synchronously such as {{HBaseAdmin#mergeRegionsSync}}. The caveat here is
> that we shouldn't use the name {{#mergeRegions}} since it exists in 1.x with
> async semantics.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
