You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@omid.apache.org by ohadshacham <gi...@git.apache.org> on 2017/11/08 12:29:25 UTC
[GitHub] incubator-omid pull request #16: [OMID-78] Identify transaction snapshot at ...
GitHub user ohadshacham opened a pull request:
https://github.com/apache/incubator-omid/pull/16
[OMID-78] Identify transaction snapshot at the server
To identify a transaction's snapshot in scans and gets, Omid reads and filters cells at the client side. This filtering requires checking the shadow cells and accessing the commit table if needed.
Doing the filtering at the server side should be more efficient since less data is being transferred to the client and also, in some cases, RPCs are being saved.
This feature requires implementing a coprocessor that will do the filtering at the server side and returns only the relevant snapshot to the client side.
This feature is also needed to support the integration of Omid with Phoenix. This is required since Phoenix has coprocessors that change and condence the data returned to the client and therefore, filtering at the client side in this case is impossible.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ohadshacham/incubator-omid ServerFiltering
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-omid/pull/16.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #16
----
commit 83de11a7e64df7dd71c14f830fa96f8ec1646812
Author: Ohad Shacham <oh...@yahoo-inc.com>
Date: 2017-10-26T08:36:04Z
Refactor the code to extract the filtering to a different object.
The intention of the extraction is to share code between the client and server side filtering.
commit d49ff5b10fd4b35a740525e5ea9146e9cba48788
Author: Ohad Shacham <oh...@yahoo-inc.com>
Date: 2017-10-30T08:55:42Z
intermediate version
commit d5ea6a23e00b02260f8708172bc505ac4a5c0b1b
Author: Ohad Shacham <oh...@yahoo-inc.com>
Date: 2017-10-31T10:11:35Z
Implement get function at the coprocessor
commit 9aa6078a4821ea4e5a1e25e5a2bba2d894a3b055
Author: Ohad Shacham <oh...@yahoo-inc.com>
Date: 2017-10-31T10:53:33Z
addition
commit 11e90026749ad2ac667bcd27f3ca48878fc08ec4
Author: Ohad Shacham <oh...@yahoo-inc.com>
Date: 2017-11-01T13:04:53Z
Working get with tests
commit 51c887d4ab555318a3e70f83c2760fc0a7e5b1ad
Author: Ohad Shacham <oh...@yahoo-inc.com>
Date: 2017-11-01T14:32:50Z
Merge branch 'ServerFiltering' of https://github.com/ohadshacham/incubator-omid into ServerFiltering
commit 20077d4bacfb0ec6524c8e70e6355059be432d6f
Author: Ohad Shacham <oh...@yahoo-inc.com>
Date: 2017-11-07T15:18:10Z
Working version for hbase0
commit 8246c3c7e97474b7c7fc701c07d312093bb1cb05
Author: Ohad Shacham <oh...@yahoo-inc.com>
Date: 2017-11-08T09:50:55Z
Working version for hbase-0 and hbase-1
commit ab2bb7b22535b7fa165d2cc2c0dbacd4c2f28009
Author: Ohad Shacham <oh...@yahoo-inc.com>
Date: 2017-11-08T11:02:26Z
Let two mini clusters run concurrently.
----
---
[GitHub] incubator-omid pull request #16: [OMID-78] Identify transaction snapshot at ...
Posted by ohadshacham <gi...@git.apache.org>.
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/16#discussion_r150795933
--- Diff: hbase-client/src/main/java/org/apache/omid/transaction/HTableAccessWrapper.java ---
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.omid.transaction;
+
+import java.io.IOException;
+import java.util.List;
+
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.HTableInterface;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.Result;
+
+public class HTableAccessWrapper implements TableAccessWrapper {
+
+ private final HTableInterface healerTable;
--- End diff --
Done
---
[GitHub] incubator-omid pull request #16: [OMID-78] Identify transaction snapshot at ...
Posted by ohadshacham <gi...@git.apache.org>.
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/16#discussion_r150795077
--- Diff: hbase-coprocessor/src/main/java/org/apache/hadoop/hbase/regionserver/RegionAccessWrapper.java ---
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.io.IOException;
+import java.util.List;
+
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.HTableInterface;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.omid.transaction.TableAccessWrapper;
+
+public class RegionAccessWrapper implements TableAccessWrapper {
--- End diff --
done
---
[GitHub] incubator-omid pull request #16: [OMID-78] Identify transaction snapshot at ...
Posted by ohadshacham <gi...@git.apache.org>.
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/16#discussion_r150794314
--- Diff: hbase-coprocessor/src/main/java/org/apache/hadoop/hbase/regionserver/OmidRegionScanner.java ---
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.regionserver.RegionScanner;
+import org.apache.hadoop.hbase.regionserver.ScannerContext;
+import org.apache.omid.transaction.HBaseTransaction;
+import org.apache.omid.transaction.SnapshotFilter;
+
+public class OmidRegionScanner implements RegionScanner {
+
+ RegionScanner scanner;
+ SnapshotFilter snapshotFilter;
+ HBaseTransaction transaction;
+ int maxVersions;
+ Map<String, List<Cell>> familyDeletionCache;
+
+ public OmidRegionScanner(SnapshotFilter snapshotFilter,
+ RegionScanner s,
+ HBaseTransaction transaction,
+ int maxVersions) {
+ this.snapshotFilter = snapshotFilter;
+ this.scanner = s;
+ this.transaction = transaction;
+ this.maxVersions = maxVersions;
+ this.familyDeletionCache = new HashMap<String, List<Cell>>();
+ }
+
+ @Override
+ public boolean next(List<Cell> results) throws IOException {
+ return next(results, Integer.MAX_VALUE);
+ }
+
+ public boolean next(List<Cell> result, int limit) throws IOException {
+ return nextRaw(result, limit);
+ }
+
+ @Override
+ public void close() throws IOException {
+ scanner.close();
+ }
+
+ @Override
+ public HRegionInfo getRegionInfo() {
+ return scanner.getRegionInfo();
+ }
+
+ @Override
+ public boolean isFilterDone() throws IOException {
+ return scanner.isFilterDone();
+ }
+
+ @Override
+ public boolean reseek(byte[] row) throws IOException {
+ throw new RuntimeException("Not implemented");
+ }
+
+ @Override
+ public long getMaxResultSize() {
+ return scanner.getMaxResultSize();
+ }
+
+ @Override
+ public long getMvccReadPoint() {
+ return scanner.getMvccReadPoint();
+ }
+
+ @Override
+ public boolean nextRaw(List<Cell> result) throws IOException {
--- End diff --
The function is defined in RegionScanner interface, we are just override it.
---
[GitHub] incubator-omid pull request #16: [OMID-78] Identify transaction snapshot at ...
Posted by yonigottesman <gi...@git.apache.org>.
Github user yonigottesman commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/16#discussion_r153424921
--- Diff: hbase-client/src/main/java/org/apache/omid/transaction/SnapshotFilter.java ---
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.omid.transaction;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.client.ResultScanner;
+import org.apache.hadoop.hbase.client.Scan;
+import org.apache.omid.committable.CommitTable.CommitTimestamp;
+
+import com.google.common.base.Optional;
+
+public interface SnapshotFilter {
+
+ public Result get(TTable ttable, Get get, HBaseTransaction transaction) throws IOException;
+
+ public ResultScanner getScanner(TTable ttable, Scan scan, HBaseTransaction transaction) throws IOException;
+
+ public List<Cell> filterCellsForSnapshot(List<Cell> rawCells, HBaseTransaction transaction,
+ int versionsToRequest, Map<String, List<Cell>> familyDeletionCache) throws IOException;
+
+ public boolean isCommitted(HBaseCellId hBaseCellId, long epoch) throws TransactionException;
--- End diff --
Why do all other function throw ioexceptions and this one something else?
---
[GitHub] incubator-omid pull request #16: [OMID-78] Identify transaction snapshot at ...
Posted by yonigottesman <gi...@git.apache.org>.
Github user yonigottesman commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/16#discussion_r153421145
--- Diff: hbase-client/src/main/java/org/apache/omid/transaction/TTable.java ---
@@ -92,16 +95,40 @@ public TTable(Configuration conf, String tableName) throws IOException {
this(conf, Bytes.toBytes(tableName));
}
+ public TTable(Configuration conf, String tableName, CommitTable.Client commitTableClient) throws IOException {
+ this(conf, Bytes.toBytes(tableName), commitTableClient);
+ }
+
public TTable(HTableInterface hTable) throws IOException {
table = hTable;
healerTable = new HTable(table.getConfiguration(), table.getTableName());
+ serverSideFilter = table.getConfiguration().getBoolean("omid.server.side.filter", false);
+ snapshotFilter = (serverSideFilter) ? new AttributeSetSnapshotFilter(hTable) : new SnapshotFilterImpl(new HTableAccessWrapper(hTable, healerTable));
+ }
+
+ public TTable(HTableInterface hTable, CommitTable.Client commitTableClient) throws IOException {
+ table = hTable;
+ healerTable = new HTable(table.getConfiguration(), table.getTableName());
+ serverSideFilter = table.getConfiguration().getBoolean("omid.server.side.filter", false);
--- End diff --
serverSideFilter should be local
---
[GitHub] incubator-omid pull request #16: [OMID-78] Identify transaction snapshot at ...
Posted by yonigottesman <gi...@git.apache.org>.
Github user yonigottesman commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/16#discussion_r153422231
--- Diff: hbase-coprocessor/src/main/java/org/apache/hadoop/hbase/regionserver/OmidRegionScanner.java ---
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.regionserver.RegionScanner;
+import org.apache.hadoop.hbase.regionserver.ScannerContext;
+import org.apache.omid.transaction.HBaseTransaction;
+import org.apache.omid.transaction.SnapshotFilterImpl;
+
+public class OmidRegionScanner implements RegionScanner {
+
+ RegionScanner scanner;
--- End diff --
Why are these package-private and not private?
---
[GitHub] incubator-omid pull request #16: [OMID-78] Identify transaction snapshot at ...
Posted by ohadshacham <gi...@git.apache.org>.
Github user ohadshacham closed the pull request at:
https://github.com/apache/incubator-omid/pull/16
---
[GitHub] incubator-omid pull request #16: [OMID-78] Identify transaction snapshot at ...
Posted by yonigottesman <gi...@git.apache.org>.
Github user yonigottesman commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/16#discussion_r153421050
--- Diff: hbase-client/src/main/java/org/apache/omid/transaction/TTable.java ---
@@ -76,6 +71,10 @@
private HTableInterface table;
+ private SnapshotFilter snapshotFilter;
+
+ private boolean serverSideFilter;
--- End diff --
No need to be a class field, you use it once in a function so make it local there.
---
[GitHub] incubator-omid pull request #16: [OMID-78] Identify transaction snapshot at ...
Posted by ohadshacham <gi...@git.apache.org>.
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/16#discussion_r150794375
--- Diff: hbase-client/src/main/java/org/apache/omid/transaction/HTableAccessWrapper.java ---
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.omid.transaction;
+
+import java.io.IOException;
+import java.util.List;
+
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.HTableInterface;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.Result;
+
+public class HTableAccessWrapper implements TableAccessWrapper {
--- End diff --
RegionAccessWrapper
---
[GitHub] incubator-omid pull request #16: [OMID-78] Identify transaction snapshot at ...
Posted by yonigottesman <gi...@git.apache.org>.
Github user yonigottesman commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/16#discussion_r153424483
--- Diff: hbase-client/src/main/java/org/apache/omid/transaction/HBaseTransactionManager.java ---
@@ -292,7 +268,7 @@ public ConflictDetectionLevel getConflictDetectionLevel() {
get.addColumn(family, shadowCellQualifier);
get.setMaxVersions(1);
get.setTimeStamp(startTimestamp);
- Result result = hBaseCellId.getTable().get(get);
+ Result result = (tableAccessWrapper != null) ? tableAccessWrapper.get(get) : hBaseCellId.getTable().get(get);
--- End diff --
why should this be null? Maybe its better to pass "hBaseCellId.getTable().get(get);" to the constructor code
---
[GitHub] incubator-omid pull request #16: [OMID-78] Identify transaction snapshot at ...
Posted by ohadshacham <gi...@git.apache.org>.
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/16#discussion_r150794291
--- Diff: hbase-coprocessor/src/main/java/org/apache/hadoop/hbase/regionserver/OmidRegionScanner.java ---
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.regionserver.RegionScanner;
+import org.apache.hadoop.hbase.regionserver.ScannerContext;
+import org.apache.omid.transaction.HBaseTransaction;
+import org.apache.omid.transaction.SnapshotFilter;
+
+public class OmidRegionScanner implements RegionScanner {
+
+ RegionScanner scanner;
+ SnapshotFilter snapshotFilter;
+ HBaseTransaction transaction;
+ int maxVersions;
+ Map<String, List<Cell>> familyDeletionCache;
+
+ public OmidRegionScanner(SnapshotFilter snapshotFilter,
+ RegionScanner s,
+ HBaseTransaction transaction,
+ int maxVersions) {
+ this.snapshotFilter = snapshotFilter;
+ this.scanner = s;
+ this.transaction = transaction;
+ this.maxVersions = maxVersions;
+ this.familyDeletionCache = new HashMap<String, List<Cell>>();
+ }
+
+ @Override
+ public boolean next(List<Cell> results) throws IOException {
+ return next(results, Integer.MAX_VALUE);
+ }
+
+ public boolean next(List<Cell> result, int limit) throws IOException {
--- End diff --
The function is defined in RegionScanner interface, we are just override it.
---