[
https://issues.apache.org/jira/browse/HBASE-9612?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13786380#comment-13786380
]
stack commented on HBASE-9612:
------------------------------
Can we get this into your patch?
{code}
1 List<Action<Row>> toReplay = new
ArrayList<Action<Row>>(initialActions.size());
2 - for (List<Action<Row>> actions : rsActions.actions.values()) {
3 - for (Action<Row> action : actions) {
4 + for (Map.Entry<byte [], List<Action<Row>>> e :
rsActions.getActions().entrySet()) {
5 + for (Action<Row> action : e.getValue()) {
{code}
The call to actions.values is expensive -- it makes new list of all actions
iterating over total set.
+1 on change to Get.
closest flag needs to move from GetRequest into the Get proto. This...
3 + // If the row to get doesn't exist, return the closest row before.
2 + optional bool closest_row_before = 10;
Remove this comment that is now wrong:
6 - * Unless existence_only is specified in the get, return all the requested
data
7 - * for the row that matches exactly, or the one that immediately
8 - * precedes it if closest_row_before is specified.
This is interesting:
12 message RegionActionResult {
11 - repeated ResultOrException resultOrException = 2;
10 - // If the operation failed globally for this region, this exception is
set
9 - optional NameBytesPair exception = 3;
I like the way you added this new exceptoin and your use of it in HRS.
In ResponseConverter I was returning an empty Result. You are throwing an
exception. Lets keep yours for now (You are the '-' and I am the '+' in below).
{code}
31 - // no result & no exception. Unexpected.
32 - throw new IllegalStateException("No result & no exception roe="
+ roe +
33 - " for region " + actions.getRegion());
34 + // Just a placeholder
35 + results.add(regionName, new Pair<Integer,
Object>(roe.getIndex(), new Result()));
36 }
37 }
38 }
{code}
You don't need this change in yours?
{code}
20 diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
21 index af522b1..46e181f 100644
22 ---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
23 +++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
24 @@ -4357,9 +4357,10 @@ public class TestFromClientSide {
25 arm.add(d);
26 // TODO: Trying mutateRow again. The batch was failing with a one try
only.
27 // t.mutateRow(arm);
28 - t.batch(Arrays.asList((Row)arm));
29 + t.mutateRow(arm);
{code}
Test MultiParallel was failing w/o the above.
Yeah, your patch is less change than mine. Good one [~nkeywal]
> Ability to batch edits destined to different regions
> ----------------------------------------------------
>
> Key: HBASE-9612
> URL: https://issues.apache.org/jira/browse/HBASE-9612
> Project: HBase
> Issue Type: Bug
> Affects Versions: 0.95.0, 0.95.1, 0.95.2, 0.96.0
> Reporter: Benoit Sigoure
> Assignee: stack
> Priority: Critical
> Fix For: 0.98.0, 0.96.0
>
> Attachments:
> 0001-fix-packaging-by-region-in-MultiServerCallable.patch, 9612.096.v5.txt,
> 9612revert.txt, 9612-v10-nico.patch, 9612v10.txt, 9612-v11-nico.patch,
> 9612v11.txt, 9612v12.txt, 9612v2.txt, 9612v3.txt, 9612v4.txt, 9612v5.txt,
> 9612v5.txt, 9612v5.txt, 9612v7.txt, 9612v8.096.txt, 9612v8.txt, 9612v9.txt,
> 9612v9.txt, 9612.wip.txt
>
>
> The old (pre-PB) "multi" and "multiPut" RPCs allowed one to batch edits
> destined to different regions. Seems like we've lost this ability after the
> switch to protobufs.
> The {{MultiRequest}} only contains one {{RegionSpecifier}}, and a list of
> {{MultiAction}}. The {{MultiAction}} message is contains either a single
> {{MutationProto}} or a {{Get}} (but not both – so its name is misleading as
> there is nothing "multi" about it). Also it seems redundant with
> {{MultiGetRequest}}, I'm not sure what's the point of supporting {{Get}} in
> {{MultiAction}}.
> I propose that we change {{MultiRequest}} to be a just a list of
> {{MultiAction}}, and {{MultiAction}} will contain the {{RegionSpecifier}},
> the {{bool atomic}} and a list of {{MutationProto}}. This would be a
> non-backward compatible protobuf change.
> If we want we can support mixing edits and reads, in which case we'd also add
> a list of {{Get}} in {{MultiAction}}, and we'd have support having both that
> list and the list of {{MutationProto}} set at the same time. But this is a
> bonus and can be done later (in a backward compatible manner, hence no need
> to rush on this one).
--
This message was sent by Atlassian JIRA
(v6.1#6144)
