[GitHub] incubator-omid issue #41: [OMID-102] Support for user Filter when using copr...
Github user JamesRTaylor commented on the issue: https://github.com/apache/incubator-omid/pull/41 This is great, @yonigottesman! Do the Phoenix unit tests FlappingTransactionIT.testInflightUpdateNotSeen() and testInflightDeleteNotSeen() pass with this change? You can try running them from the omid2 feature branch. ---
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206935434
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
+
+public SnapshotFilter getSnapshotFilter() {
+return snapshotFilter;
+}
+
+public TransactionVisibilityFilter(@Nullable Filter cellFilter,
+ SnapshotFilterImpl snapshotFilter,
+ HBaseTransaction hbaseTransaction) {
+this.userFilter = cellFilter;
+this.snapshotFilter = snapshotFilter;
+shadowCellCache = new HashMap<>();
+this.hbaseTransaction = hbaseTransaction;
+familyDeletionCache = new HashMap();
+}
+
+@Override
+public ReturnCode filterKeyValue(Cell v) throws IOException {
+if (CellUtils.isShadowCell(v)) {
+Long commitTs = Bytes.toLong(CellUtil.cloneValue(v));
+shadowCellCache.put(v.getTimestamp(), commitTs);
+// Continue getting shadow cells until one of them fits this
transaction
+if (hbaseTransaction.getStartTimestamp() >= commitTs) {
+return ReturnCode.NEXT_COL;
+} else {
+return ReturnCode.SKIP;
+}
+} else if (CellUtils.isFamilyDeleteCell(v)) {
+//Delete is part of this transaction
+if (snapshotFilter.getTSIfInTransaction(v,
hbaseTransaction).isPresent()) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
v.getTimestamp());
+return ReturnCode.NEXT_COL;
+}
+
+if (shadowCellCache.containsKey(v.getTimestamp()) &&
+hbaseTransaction.getStartTimestamp() >=
shadowCellCache.get(v.getTimestamp())) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
shadowCellCache.get(v.getTimestamp()));
+return ReturnCode.NEXT_COL;
+}
+
+// Try to get shadow cell from region
+final Get get = new Get(CellUtil.cloneRow(v));
+get.setTimeStamp(v.getTimestamp()).setMaxVersions(1);
+get.addColumn(CellUtil.cloneFamily(v),
CellUtils.addShadowCellSuffix(CellUtils.FAMILY_DELETE_QUALIFIER));
+Result deleteFamilySC =
snapshotFilter.getTableAccessWrapper().get(get);
+
+if (!deleteFamilySC.isEmpty() &&
+
Bytes.toLong(CellUtil.cloneValue(deleteFamilySC.rawCells()[0] )) <
hbaseTransaction.getStartTimestamp()){
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
Bytes.toLong(CellUtil.cloneValue(deleteFamilySC.rawCells()[0])));
+return ReturnCode.NEXT_COL;
+}
+
+//At last go to commit table
+Optional
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206934998
--- Diff:
hbase-coprocessor/src/test/java/org/apache/omid/transaction/TestSnapshotFilter.java
---
@@ -226,8 +222,115 @@ public void testGetFirstResult() throws Throwable {
}
@Test(timeOut = 60_000)
-public void testGetSecondResult() throws Throwable {
--- End diff --
Would be good to have a test that uses a scan with FirstKeyOnlyFilter that
would get incorrect results without this new implementation.
---
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206936274
--- Diff:
hbase-common/src/main/java/org/apache/omid/transaction/CellUtils.java ---
@@ -52,6 +52,7 @@
static byte[] DELETE_TOMBSTONE = Bytes.toBytes("__OMID_TOMBSTONE__");
--- End diff --
Will Cells end up with these constant values and if so can we make them
shorter?
---
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206860096
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
--- End diff --
I would add a comment that the row info is redundant in here since reset is
called between rows and we clear this map in reset.
---
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206861622
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
+
+public SnapshotFilter getSnapshotFilter() {
+return snapshotFilter;
+}
+
+public TransactionVisibilityFilter(@Nullable Filter cellFilter,
+ SnapshotFilterImpl snapshotFilter,
+ HBaseTransaction hbaseTransaction) {
+this.userFilter = cellFilter;
+this.snapshotFilter = snapshotFilter;
+shadowCellCache = new HashMap<>();
+this.hbaseTransaction = hbaseTransaction;
+familyDeletionCache = new HashMap();
+}
+
+@Override
+public ReturnCode filterKeyValue(Cell v) throws IOException {
+if (CellUtils.isShadowCell(v)) {
+Long commitTs = Bytes.toLong(CellUtil.cloneValue(v));
+shadowCellCache.put(v.getTimestamp(), commitTs);
+// Continue getting shadow cells until one of them fits this
transaction
+if (hbaseTransaction.getStartTimestamp() >= commitTs) {
+return ReturnCode.NEXT_COL;
+} else {
+return ReturnCode.SKIP;
+}
+} else if (CellUtils.isFamilyDeleteCell(v)) {
+//Delete is part of this transaction
+if (snapshotFilter.getTSIfInTransaction(v,
hbaseTransaction).isPresent()) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
v.getTimestamp());
+return ReturnCode.NEXT_COL;
+}
+
+if (shadowCellCache.containsKey(v.getTimestamp()) &&
+hbaseTransaction.getStartTimestamp() >=
shadowCellCache.get(v.getTimestamp())) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
shadowCellCache.get(v.getTimestamp()));
+return ReturnCode.NEXT_COL;
+}
+
+// Try to get shadow cell from region
+final Get get = new Get(CellUtil.cloneRow(v));
+get.setTimeStamp(v.getTimestamp()).setMaxVersions(1);
+get.addColumn(CellUtil.cloneFamily(v),
CellUtils.addShadowCellSuffix(CellUtils.FAMILY_DELETE_QUALIFIER));
+Result deleteFamilySC =
snapshotFilter.getTableAccessWrapper().get(get);
+
+if (!deleteFamilySC.isEmpty() &&
+
Bytes.toLong(CellUtil.cloneValue(deleteFamilySC.rawCells()[0] )) <
hbaseTransaction.getStartTimestamp()){
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
Bytes.toLong(CellUtil.cloneValue(deleteFamilySC.rawCells()[0])));
+return ReturnCode.NEXT_COL;
+}
+
+//At last go to commit table
+Optional commitTimestamp
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206848753
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/OmidSnapshotFilter.java
---
@@ -83,92 +76,97 @@ public void start(CoprocessorEnvironment env) throws
IOException {
if (commitTableName != null) {
commitTableConf.setTableName(commitTableName);
}
-if (commitTableClient == null) {
-commitTableClient = initAndGetCommitTableClient();
-}
-snapshotFilter = new SnapshotFilterImpl(commitTableClient);
-
LOG.info("Snapshot filter started");
}
@Override
public void stop(CoprocessorEnvironment e) throws IOException {
LOG.info("Stopping snapshot filter coprocessor");
-commitTableClient.close();
+if (snapshotFilterQueue != null) {
+for (SnapshotFilter snapshotFilter: snapshotFilterQueue) {
+
((SnapshotFilterImpl)snapshotFilter).getCommitTableClient().close();
+}
+}
LOG.info("Snapshot filter stopped");
}
-public void setCommitTableClient(CommitTable.Client commitTableClient)
{
-this.commitTableClient = commitTableClient;
-this.snapshotFilter.setCommitTableClient(commitTableClient);
-}
@Override
-public void preGetOp(ObserverContext c,
Get get, List result) throws IOException {
-
-if (get.getAttribute(CellUtils.CLIENT_GET_ATTRIBUTE) == null)
return;
-
-try {
-get.setAttribute(CellUtils.CLIENT_GET_ATTRIBUTE, null);
-RegionAccessWrapper regionAccessWrapper = new
RegionAccessWrapper(HBaseShims.getRegionCoprocessorRegion(c.getEnvironment()));
-Result res = regionAccessWrapper.get(get); // get parameters
were set at the client side
-
-snapshotFilter.setTableAccessWrapper(regionAccessWrapper);
+public void postGetOp(ObserverContext e,
Get get, List results)
+throws IOException {
+if (get.getFilter() != null) {
+//This get had a filter and used a commit table client that
must put back
+assert (get.getFilter() instanceof
TransactionVisibilityFilter);
+TransactionVisibilityFilter filter =
(TransactionVisibilityFilter)get.getFilter();
+
snapshotFilterQueue.add((SnapshotFilterImpl)filter.getSnapshotFilter());
+}
+}
-List filteredKeyValues = Collections.emptyList();
-if (!res.isEmpty()) {
-TSOProto.Transaction transaction =
TSOProto.Transaction.parseFrom(get.getAttribute(CellUtils.TRANSACTION_ATTRIBUTE));
-long id = transaction.getTimestamp();
-long readTs = transaction.getReadTimestamp();
-long epoch = transaction.getEpoch();
-VisibilityLevel visibilityLevel =
VisibilityLevel.fromInteger(transaction.getVisibilityLevel());
+@Override
+public void preGetOp(ObserverContext e,
Get get, List results)
+throws IOException {
+if (get.getAttribute(CellUtils.CLIENT_GET_ATTRIBUTE) == null)
return;
-HBaseTransaction hbaseTransaction = new
HBaseTransaction(id, readTs, visibilityLevel, epoch, new
HashSet(), new HashSet(), null);
-filteredKeyValues =
snapshotFilter.filterCellsForSnapshot(res.listCells(), hbaseTransaction,
get.getMaxVersions(), new HashMap(), get.getAttributesMap());
-}
+HBaseTransaction hbaseTransaction =
getHBaseTransaction(get.getAttribute(CellUtils.TRANSACTION_ATTRIBUTE));
+SnapshotFilterImpl snapshotFilter = getSnapshotFilter(e);
+get.setMaxVersions();
--- End diff --
I would add a comment that we set to max versions since we are doing Omid
filtering in the VisibilityFilter
---
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206867892
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
+
+public SnapshotFilter getSnapshotFilter() {
+return snapshotFilter;
+}
+
+public TransactionVisibilityFilter(@Nullable Filter cellFilter,
+ SnapshotFilterImpl snapshotFilter,
+ HBaseTransaction hbaseTransaction) {
+this.userFilter = cellFilter;
+this.snapshotFilter = snapshotFilter;
+shadowCellCache = new HashMap<>();
+this.hbaseTransaction = hbaseTransaction;
+familyDeletionCache = new HashMap();
+}
+
+@Override
+public ReturnCode filterKeyValue(Cell v) throws IOException {
+if (CellUtils.isShadowCell(v)) {
+Long commitTs = Bytes.toLong(CellUtil.cloneValue(v));
+shadowCellCache.put(v.getTimestamp(), commitTs);
+// Continue getting shadow cells until one of them fits this
transaction
+if (hbaseTransaction.getStartTimestamp() >= commitTs) {
+return ReturnCode.NEXT_COL;
+} else {
+return ReturnCode.SKIP;
+}
+} else if (CellUtils.isFamilyDeleteCell(v)) {
+//Delete is part of this transaction
+if (snapshotFilter.getTSIfInTransaction(v,
hbaseTransaction).isPresent()) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
v.getTimestamp());
+return ReturnCode.NEXT_COL;
+}
+
+if (shadowCellCache.containsKey(v.getTimestamp()) &&
+hbaseTransaction.getStartTimestamp() >=
shadowCellCache.get(v.getTimestamp())) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
shadowCellCache.get(v.getTimestamp()));
+return ReturnCode.NEXT_COL;
+}
+
+// Try to get shadow cell from region
+final Get get = new Get(CellUtil.cloneRow(v));
+get.setTimeStamp(v.getTimestamp()).setMaxVersions(1);
+get.addColumn(CellUtil.cloneFamily(v),
CellUtils.addShadowCellSuffix(CellUtils.FAMILY_DELETE_QUALIFIER));
+Result deleteFamilySC =
snapshotFilter.getTableAccessWrapper().get(get);
+
+if (!deleteFamilySC.isEmpty() &&
+
Bytes.toLong(CellUtil.cloneValue(deleteFamilySC.rawCells()[0] )) <
hbaseTransaction.getStartTimestamp()){
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
Bytes.toLong(CellUtil.cloneValue(deleteFamilySC.rawCells()[0])));
+return ReturnCode.NEXT_COL;
+}
+
+//At last go to commit table
+Optional commitTimestamp
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206927180
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
+
+public SnapshotFilter getSnapshotFilter() {
+return snapshotFilter;
+}
+
+public TransactionVisibilityFilter(@Nullable Filter cellFilter,
+ SnapshotFilterImpl snapshotFilter,
+ HBaseTransaction hbaseTransaction) {
+this.userFilter = cellFilter;
+this.snapshotFilter = snapshotFilter;
+shadowCellCache = new HashMap<>();
+this.hbaseTransaction = hbaseTransaction;
+familyDeletionCache = new HashMap();
+}
+
+@Override
+public ReturnCode filterKeyValue(Cell v) throws IOException {
+if (CellUtils.isShadowCell(v)) {
+Long commitTs = Bytes.toLong(CellUtil.cloneValue(v));
+shadowCellCache.put(v.getTimestamp(), commitTs);
+// Continue getting shadow cells until one of them fits this
transaction
+if (hbaseTransaction.getStartTimestamp() >= commitTs) {
+return ReturnCode.NEXT_COL;
+} else {
+return ReturnCode.SKIP;
+}
+} else if (CellUtils.isFamilyDeleteCell(v)) {
+//Delete is part of this transaction
+if (snapshotFilter.getTSIfInTransaction(v,
hbaseTransaction).isPresent()) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
v.getTimestamp());
+return ReturnCode.NEXT_COL;
+}
+
+if (shadowCellCache.containsKey(v.getTimestamp()) &&
+hbaseTransaction.getStartTimestamp() >=
shadowCellCache.get(v.getTimestamp())) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
shadowCellCache.get(v.getTimestamp()));
--- End diff --
Avoid allocating memory within this call. Either use TreeMap
and don't do the copy or use HashMap. You can
copy/paste ImmutableBytesPtr from Phoenix - it's a wrapper around byte[] that
handles equality and hashcode without doing any copying.
---
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206856182
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
+
+public SnapshotFilter getSnapshotFilter() {
+return snapshotFilter;
+}
+
+public TransactionVisibilityFilter(@Nullable Filter cellFilter,
+ SnapshotFilterImpl snapshotFilter,
+ HBaseTransaction hbaseTransaction) {
+this.userFilter = cellFilter;
+this.snapshotFilter = snapshotFilter;
+shadowCellCache = new HashMap<>();
+this.hbaseTransaction = hbaseTransaction;
+familyDeletionCache = new HashMap();
+}
+
+@Override
+public ReturnCode filterKeyValue(Cell v) throws IOException {
+if (CellUtils.isShadowCell(v)) {
+Long commitTs = Bytes.toLong(CellUtil.cloneValue(v));
+shadowCellCache.put(v.getTimestamp(), commitTs);
+// Continue getting shadow cells until one of them fits this
transaction
+if (hbaseTransaction.getStartTimestamp() >= commitTs) {
+return ReturnCode.NEXT_COL;
+} else {
+return ReturnCode.SKIP;
+}
+} else if (CellUtils.isFamilyDeleteCell(v)) {
+//Delete is part of this transaction
+if (snapshotFilter.getTSIfInTransaction(v,
hbaseTransaction).isPresent()) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
v.getTimestamp());
+return ReturnCode.NEXT_COL;
+}
+
+if (shadowCellCache.containsKey(v.getTimestamp()) &&
--- End diff --
You can do a get and check the existence using the return value. It saves a
collection access.
---
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206857467
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
+
+public SnapshotFilter getSnapshotFilter() {
+return snapshotFilter;
+}
+
+public TransactionVisibilityFilter(@Nullable Filter cellFilter,
+ SnapshotFilterImpl snapshotFilter,
+ HBaseTransaction hbaseTransaction) {
+this.userFilter = cellFilter;
+this.snapshotFilter = snapshotFilter;
+shadowCellCache = new HashMap<>();
+this.hbaseTransaction = hbaseTransaction;
+familyDeletionCache = new HashMap();
+}
+
+@Override
+public ReturnCode filterKeyValue(Cell v) throws IOException {
+if (CellUtils.isShadowCell(v)) {
+Long commitTs = Bytes.toLong(CellUtil.cloneValue(v));
+shadowCellCache.put(v.getTimestamp(), commitTs);
+// Continue getting shadow cells until one of them fits this
transaction
+if (hbaseTransaction.getStartTimestamp() >= commitTs) {
+return ReturnCode.NEXT_COL;
+} else {
+return ReturnCode.SKIP;
+}
+} else if (CellUtils.isFamilyDeleteCell(v)) {
+//Delete is part of this transaction
+if (snapshotFilter.getTSIfInTransaction(v,
hbaseTransaction).isPresent()) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
v.getTimestamp());
+return ReturnCode.NEXT_COL;
+}
+
+if (shadowCellCache.containsKey(v.getTimestamp()) &&
+hbaseTransaction.getStartTimestamp() >=
shadowCellCache.get(v.getTimestamp())) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
shadowCellCache.get(v.getTimestamp()));
+return ReturnCode.NEXT_COL;
+}
+
+// Try to get shadow cell from region
+final Get get = new Get(CellUtil.cloneRow(v));
+get.setTimeStamp(v.getTimestamp()).setMaxVersions(1);
+get.addColumn(CellUtil.cloneFamily(v),
CellUtils.addShadowCellSuffix(CellUtils.FAMILY_DELETE_QUALIFIER));
--- End diff --
Change the name of addShadowCellSuffix to addShadowCellMetadata?
---
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206862264
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
+
+public SnapshotFilter getSnapshotFilter() {
+return snapshotFilter;
+}
+
+public TransactionVisibilityFilter(@Nullable Filter cellFilter,
+ SnapshotFilterImpl snapshotFilter,
+ HBaseTransaction hbaseTransaction) {
+this.userFilter = cellFilter;
+this.snapshotFilter = snapshotFilter;
+shadowCellCache = new HashMap<>();
+this.hbaseTransaction = hbaseTransaction;
+familyDeletionCache = new HashMap();
+}
+
+@Override
+public ReturnCode filterKeyValue(Cell v) throws IOException {
+if (CellUtils.isShadowCell(v)) {
+Long commitTs = Bytes.toLong(CellUtil.cloneValue(v));
+shadowCellCache.put(v.getTimestamp(), commitTs);
+// Continue getting shadow cells until one of them fits this
transaction
+if (hbaseTransaction.getStartTimestamp() >= commitTs) {
+return ReturnCode.NEXT_COL;
+} else {
+return ReturnCode.SKIP;
+}
+} else if (CellUtils.isFamilyDeleteCell(v)) {
+//Delete is part of this transaction
+if (snapshotFilter.getTSIfInTransaction(v,
hbaseTransaction).isPresent()) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
v.getTimestamp());
+return ReturnCode.NEXT_COL;
+}
+
+if (shadowCellCache.containsKey(v.getTimestamp()) &&
+hbaseTransaction.getStartTimestamp() >=
shadowCellCache.get(v.getTimestamp())) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
shadowCellCache.get(v.getTimestamp()));
+return ReturnCode.NEXT_COL;
+}
+
+// Try to get shadow cell from region
+final Get get = new Get(CellUtil.cloneRow(v));
+get.setTimeStamp(v.getTimestamp()).setMaxVersions(1);
+get.addColumn(CellUtil.cloneFamily(v),
CellUtils.addShadowCellSuffix(CellUtils.FAMILY_DELETE_QUALIFIER));
+Result deleteFamilySC =
snapshotFilter.getTableAccessWrapper().get(get);
+
+if (!deleteFamilySC.isEmpty() &&
+
Bytes.toLong(CellUtil.cloneValue(deleteFamilySC.rawCells()[0] )) <
hbaseTransaction.getStartTimestamp()){
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
Bytes.toLong(CellUtil.cloneValue(deleteFamilySC.rawCells()[0])));
+return ReturnCode.NEXT_COL;
+}
+
+//At last go to commit table
+Optional commitTimestamp
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206843754
--- Diff:
hbase-common/src/main/java/org/apache/omid/transaction/CellUtils.java ---
@@ -382,13 +385,16 @@ public int hashCode() {
hasher.putBytes(cell.getRowArray(), cell.getRowOffset(),
cell.getRowLength());
hasher.putBytes(cell.getFamilyArray(), cell.getFamilyOffset(),
cell.getFamilyLength());
int qualifierLength = cell.getQualifierLength();
+int qualifierOffset = cell.getQualifierOffset();
if (isShadowCell()) { // Update qualifier length when
qualifier is shadow cell
qualifierLength =
qualifierLengthFromShadowCellQualifier(cell.getQualifierArray(),
cell.getQualifierOffset(),
cell.getQualifierLength());
+qualifierOffset = qualifierOffset + 1;
--- End diff --
Will it work when the shadow cell prefix is absent? legacy data.
---
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206852241
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
+
+public SnapshotFilter getSnapshotFilter() {
+return snapshotFilter;
+}
+
+public TransactionVisibilityFilter(@Nullable Filter cellFilter,
+ SnapshotFilterImpl snapshotFilter,
+ HBaseTransaction hbaseTransaction) {
+this.userFilter = cellFilter;
+this.snapshotFilter = snapshotFilter;
+shadowCellCache = new HashMap<>();
+this.hbaseTransaction = hbaseTransaction;
+familyDeletionCache = new HashMap();
+}
+
+@Override
+public ReturnCode filterKeyValue(Cell v) throws IOException {
+if (CellUtils.isShadowCell(v)) {
+Long commitTs = Bytes.toLong(CellUtil.cloneValue(v));
+shadowCellCache.put(v.getTimestamp(), commitTs);
+// Continue getting shadow cells until one of them fits this
transaction
+if (hbaseTransaction.getStartTimestamp() >= commitTs) {
--- End diff --
Why do we keep ones that committed after the transaction timestamp?
---
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206839768
--- Diff:
hbase-client/src/test/java/org/apache/omid/transaction/TestCellUtils.java ---
@@ -99,11 +99,11 @@ public void testShadowCellQualifiers(byte[]
shadowCellSuffixToTest) throws IOExc
public void testCorrectMapingOfCellsToShadowCells() throws IOException
{
// Create the required data
final byte[] validShadowCellQualifier =
-com.google.common.primitives.Bytes.concat(qualifier,
SHADOW_CELL_SUFFIX);
+com.google.common.primitives.Bytes.concat(new byte[1],
qualifier, SHADOW_CELL_SUFFIX);
--- End diff --
Define SHADOW_CELL_PREFIX to "new byte[1]" and replace all the new byte[1].
---
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
GitHub user yonigottesman reopened a pull request: https://github.com/apache/incubator-omid/pull/41 [OMID-102] Support for user Filter when using coprocessor for snapshot filtering You can merge this pull request into a Git repository by running: $ git pull https://github.com/yonigottesman/incubator-omid scan_filters Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-omid/pull/41.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 #41 commit 41554ec4c3cdfdbfd271e9adbef866b3653c3382 Author: Yonatan Gottesman Date: 2018-07-25T06:40:14Z Support for user Filter when using coprocessor for snapshot filtering commit ba575384dcdca427bf80e55dcee58a1ed93bf744 Author: Yonatan Gottesman Date: 2018-07-25T07:41:42Z In coprocessor filtering, get shadow cell of delete family before going to commit table commit 26469a0893b4c9bc36e68d66583951fb6e82cf9d Author: Yonatan Gottesman Date: 2018-07-26T08:15:27Z add inMemoryCommitTable client option in omid coprocessor for testing commit 8d632670ab580ca04e0c5cec555211d1fa71a11a Author: Yonatan Gottesman Date: 2018-07-31T11:29:13Z Fix visibilityFilter to check if delete family is in current TX commit b2980c7cea2ef9c83a772d4ce3bd9a82f5ae4d81 Author: Yonatan Gottesman Date: 2018-08-01T09:34:00Z Merge OMID-105 changes ---
[GitHub] incubator-omid pull request #41: [OMID-102] Support for user Filter when usi...
Github user yonigottesman closed the pull request at: https://github.com/apache/incubator-omid/pull/41 ---
[GitHub] incubator-omid pull request #40: [OMID-106] Delete should use write timestam...
Github user ohadshacham closed the pull request at: https://github.com/apache/incubator-omid/pull/40 ---
[jira] [Commented] (OMID-102) Implement visibility filter as pure HBase Filter
[ https://issues.apache.org/jira/browse/OMID-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16565538#comment-16565538 ] ASF GitHub Bot commented on OMID-102: - Github user JamesRTaylor commented on the issue: https://github.com/apache/incubator-omid/pull/41 This is great, @yonigottesman! Do the Phoenix unit tests FlappingTransactionIT.testInflightUpdateNotSeen() and testInflightDeleteNotSeen() pass with this change? You can try running them from the omid2 feature branch. > Implement visibility filter as pure HBase Filter > > > Key: OMID-102 > URL: https://issues.apache.org/jira/browse/OMID-102 > Project: Apache Omid > Issue Type: Sub-task >Reporter: James Taylor >Assignee: Yonatan Gottesman >Priority: Major > > The way Omid currently filters through it's own RegionScanner won't work the > way it's implemented (i.e. the way the filtering is done *after* the next > call). The reason is that the state of HBase filters get messed up since > these filters will start to see cells that it shouldn't (i.e. cells that > would be filtered based on snapshot isolation). It cannot be worked around by > manually running filters afterwards because filters may issue seek calls > which are handled during the running of scans by HBase. > > Instead, the filtering needs to be implemented as a pure HBase filter and > that filter needs to delegate to the other, delegate filter once it's > determined that the cell is visible. See Tephra's TransactionVisibilityFilter > and they way it calls the delegate filter (cellFilters) only after it's > determined that the cell is visible. You may run into TEPHRA-169 without > including the CellSkipFilter too. > Because it'll be easier if you see shadow cells *before* their corresponding > real cells you can prefix instead of suffix the column qualifiers to > guarantee that you'd see the shadow cells prior to the actual cells. Or you > could buffer cells in your filter prior to omitting them. Another issue would > be if the shadow cells aren't found and you need to consult the commit table > - I suppose if the shadow cells are first, this logic would be easier to know > when it needs to be called. > > To reproduce, see the Phoenix unit tests > FlappingTransactionIT.testInflightUpdateNotSeen() and > testInflightDeleteNotSeen(). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OMID-102) Implement visibility filter as pure HBase Filter
[
https://issues.apache.org/jira/browse/OMID-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16565535#comment-16565535
]
ASF GitHub Bot commented on OMID-102:
-
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206936274
--- Diff:
hbase-common/src/main/java/org/apache/omid/transaction/CellUtils.java ---
@@ -52,6 +52,7 @@
static byte[] DELETE_TOMBSTONE = Bytes.toBytes("__OMID_TOMBSTONE__");
--- End diff --
Will Cells end up with these constant values and if so can we make them
shorter?
> Implement visibility filter as pure HBase Filter
>
>
> Key: OMID-102
> URL: https://issues.apache.org/jira/browse/OMID-102
> Project: Apache Omid
> Issue Type: Sub-task
>Reporter: James Taylor
>Assignee: Yonatan Gottesman
>Priority: Major
>
> The way Omid currently filters through it's own RegionScanner won't work the
> way it's implemented (i.e. the way the filtering is done *after* the next
> call). The reason is that the state of HBase filters get messed up since
> these filters will start to see cells that it shouldn't (i.e. cells that
> would be filtered based on snapshot isolation). It cannot be worked around by
> manually running filters afterwards because filters may issue seek calls
> which are handled during the running of scans by HBase.
>
> Instead, the filtering needs to be implemented as a pure HBase filter and
> that filter needs to delegate to the other, delegate filter once it's
> determined that the cell is visible. See Tephra's TransactionVisibilityFilter
> and they way it calls the delegate filter (cellFilters) only after it's
> determined that the cell is visible. You may run into TEPHRA-169 without
> including the CellSkipFilter too.
> Because it'll be easier if you see shadow cells *before* their corresponding
> real cells you can prefix instead of suffix the column qualifiers to
> guarantee that you'd see the shadow cells prior to the actual cells. Or you
> could buffer cells in your filter prior to omitting them. Another issue would
> be if the shadow cells aren't found and you need to consult the commit table
> - I suppose if the shadow cells are first, this logic would be easier to know
> when it needs to be called.
>
> To reproduce, see the Phoenix unit tests
> FlappingTransactionIT.testInflightUpdateNotSeen() and
> testInflightDeleteNotSeen().
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OMID-102) Implement visibility filter as pure HBase Filter
[
https://issues.apache.org/jira/browse/OMID-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16565532#comment-16565532
]
ASF GitHub Bot commented on OMID-102:
-
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206935434
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
+
+public SnapshotFilter getSnapshotFilter() {
+return snapshotFilter;
+}
+
+public TransactionVisibilityFilter(@Nullable Filter cellFilter,
+ SnapshotFilterImpl snapshotFilter,
+ HBaseTransaction hbaseTransaction) {
+this.userFilter = cellFilter;
+this.snapshotFilter = snapshotFilter;
+shadowCellCache = new HashMap<>();
+this.hbaseTransaction = hbaseTransaction;
+familyDeletionCache = new HashMap();
+}
+
+@Override
+public ReturnCode filterKeyValue(Cell v) throws IOException {
+if (CellUtils.isShadowCell(v)) {
+Long commitTs = Bytes.toLong(CellUtil.cloneValue(v));
+shadowCellCache.put(v.getTimestamp(), commitTs);
+// Continue getting shadow cells until one of them fits this
transaction
+if (hbaseTransaction.getStartTimestamp() >= commitTs) {
+return ReturnCode.NEXT_COL;
+} else {
+return ReturnCode.SKIP;
+}
+} else if (CellUtils.isFamilyDeleteCell(v)) {
+//Delete is part of this transaction
+if (snapshotFilter.getTSIfInTransaction(v,
hbaseTransaction).isPresent()) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
v.getTimestamp());
+return ReturnCode.NEXT_COL;
+}
+
+if (shadowCellCache.containsKey(v.getTimestamp()) &&
+hbaseTransaction.getStartTimestamp() >=
shadowCellCache.get(v.getTimestamp())) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
shadowCellCache.get(v.getTimestamp()));
+return ReturnCode.NEXT_COL;
+}
+
+// Try to get shadow cell from region
+final Get get = new Get(CellUtil.cloneRow(v));
+get.setTimeStamp(v.getTimestamp()).setMaxVersions(1);
+get.addColumn(CellUtil.cloneFamily(v),
CellUtils.addShadowCellSuffix(CellUtils.FAMILY_DELETE_QUALIFIER));
+Result deleteFamilySC =
snapshotFilter.getTableAccessWrapper().get(get);
+
+if (!deleteFamilySC.isEmpty() &&
+
Bytes.toLong(CellUtil.cloneValue(deleteFamilySC.rawCells()[0] )) <
hbaseTransaction.getStartTimestamp()){
+
[jira] [Commented] (OMID-102) Implement visibility filter as pure HBase Filter
[
https://issues.apache.org/jira/browse/OMID-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16565529#comment-16565529
]
ASF GitHub Bot commented on OMID-102:
-
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206934998
--- Diff:
hbase-coprocessor/src/test/java/org/apache/omid/transaction/TestSnapshotFilter.java
---
@@ -226,8 +222,115 @@ public void testGetFirstResult() throws Throwable {
}
@Test(timeOut = 60_000)
-public void testGetSecondResult() throws Throwable {
--- End diff --
Would be good to have a test that uses a scan with FirstKeyOnlyFilter that
would get incorrect results without this new implementation.
> Implement visibility filter as pure HBase Filter
>
>
> Key: OMID-102
> URL: https://issues.apache.org/jira/browse/OMID-102
> Project: Apache Omid
> Issue Type: Sub-task
>Reporter: James Taylor
>Assignee: Yonatan Gottesman
>Priority: Major
>
> The way Omid currently filters through it's own RegionScanner won't work the
> way it's implemented (i.e. the way the filtering is done *after* the next
> call). The reason is that the state of HBase filters get messed up since
> these filters will start to see cells that it shouldn't (i.e. cells that
> would be filtered based on snapshot isolation). It cannot be worked around by
> manually running filters afterwards because filters may issue seek calls
> which are handled during the running of scans by HBase.
>
> Instead, the filtering needs to be implemented as a pure HBase filter and
> that filter needs to delegate to the other, delegate filter once it's
> determined that the cell is visible. See Tephra's TransactionVisibilityFilter
> and they way it calls the delegate filter (cellFilters) only after it's
> determined that the cell is visible. You may run into TEPHRA-169 without
> including the CellSkipFilter too.
> Because it'll be easier if you see shadow cells *before* their corresponding
> real cells you can prefix instead of suffix the column qualifiers to
> guarantee that you'd see the shadow cells prior to the actual cells. Or you
> could buffer cells in your filter prior to omitting them. Another issue would
> be if the shadow cells aren't found and you need to consult the commit table
> - I suppose if the shadow cells are first, this logic would be easier to know
> when it needs to be called.
>
> To reproduce, see the Phoenix unit tests
> FlappingTransactionIT.testInflightUpdateNotSeen() and
> testInflightDeleteNotSeen().
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OMID-102) Implement visibility filter as pure HBase Filter
[
https://issues.apache.org/jira/browse/OMID-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16565272#comment-16565272
]
ASF GitHub Bot commented on OMID-102:
-
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206867892
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
+
+public SnapshotFilter getSnapshotFilter() {
+return snapshotFilter;
+}
+
+public TransactionVisibilityFilter(@Nullable Filter cellFilter,
+ SnapshotFilterImpl snapshotFilter,
+ HBaseTransaction hbaseTransaction) {
+this.userFilter = cellFilter;
+this.snapshotFilter = snapshotFilter;
+shadowCellCache = new HashMap<>();
+this.hbaseTransaction = hbaseTransaction;
+familyDeletionCache = new HashMap();
+}
+
+@Override
+public ReturnCode filterKeyValue(Cell v) throws IOException {
+if (CellUtils.isShadowCell(v)) {
+Long commitTs = Bytes.toLong(CellUtil.cloneValue(v));
+shadowCellCache.put(v.getTimestamp(), commitTs);
+// Continue getting shadow cells until one of them fits this
transaction
+if (hbaseTransaction.getStartTimestamp() >= commitTs) {
+return ReturnCode.NEXT_COL;
+} else {
+return ReturnCode.SKIP;
+}
+} else if (CellUtils.isFamilyDeleteCell(v)) {
+//Delete is part of this transaction
+if (snapshotFilter.getTSIfInTransaction(v,
hbaseTransaction).isPresent()) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
v.getTimestamp());
+return ReturnCode.NEXT_COL;
+}
+
+if (shadowCellCache.containsKey(v.getTimestamp()) &&
+hbaseTransaction.getStartTimestamp() >=
shadowCellCache.get(v.getTimestamp())) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
shadowCellCache.get(v.getTimestamp()));
+return ReturnCode.NEXT_COL;
+}
+
+// Try to get shadow cell from region
+final Get get = new Get(CellUtil.cloneRow(v));
+get.setTimeStamp(v.getTimestamp()).setMaxVersions(1);
+get.addColumn(CellUtil.cloneFamily(v),
CellUtils.addShadowCellSuffix(CellUtils.FAMILY_DELETE_QUALIFIER));
+Result deleteFamilySC =
snapshotFilter.getTableAccessWrapper().get(get);
+
+if (!deleteFamilySC.isEmpty() &&
+
Bytes.toLong(CellUtil.cloneValue(deleteFamilySC.rawCells()[0] )) <
hbaseTransaction.getStartTimestamp()){
+
[jira] [Commented] (OMID-102) Implement visibility filter as pure HBase Filter
[
https://issues.apache.org/jira/browse/OMID-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16565269#comment-16565269
]
ASF GitHub Bot commented on OMID-102:
-
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206860096
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
--- End diff --
I would add a comment that the row info is redundant in here since reset is
called between rows and we clear this map in reset.
> Implement visibility filter as pure HBase Filter
>
>
> Key: OMID-102
> URL: https://issues.apache.org/jira/browse/OMID-102
> Project: Apache Omid
> Issue Type: Sub-task
>Reporter: James Taylor
>Assignee: Yonatan Gottesman
>Priority: Major
>
> The way Omid currently filters through it's own RegionScanner won't work the
> way it's implemented (i.e. the way the filtering is done *after* the next
> call). The reason is that the state of HBase filters get messed up since
> these filters will start to see cells that it shouldn't (i.e. cells that
> would be filtered based on snapshot isolation). It cannot be worked around by
> manually running filters afterwards because filters may issue seek calls
> which are handled during the running of scans by HBase.
>
> Instead, the filtering needs to be implemented as a pure HBase filter and
> that filter needs to delegate to the other, delegate filter once it's
> determined that the cell is visible. See Tephra's TransactionVisibilityFilter
> and they way it calls the delegate filter (cellFilters) only after it's
> determined that the cell is visible. You may run into TEPHRA-169 without
> including the CellSkipFilter too.
> Because it'll be easier if you see shadow cells *before* their corresponding
> real cells you can prefix instead of suffix the column qualifiers to
> guarantee that you'd see the shadow cells prior to the actual cells. Or you
> could buffer cells in your filter prior to omitting them. Another issue would
> be if the shadow cells aren't found and you need to consult the commit table
> - I suppose if the shadow cells are first, this logic would be easier to know
> when it needs to be called.
>
> To reproduce, see the Phoenix unit tests
> FlappingTransactionIT.testInflightUpdateNotSeen() and
> testInflightDeleteNotSeen().
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OMID-102) Implement visibility filter as pure HBase Filter
[
https://issues.apache.org/jira/browse/OMID-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16565262#comment-16565262
]
ASF GitHub Bot commented on OMID-102:
-
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206852241
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
+
+public SnapshotFilter getSnapshotFilter() {
+return snapshotFilter;
+}
+
+public TransactionVisibilityFilter(@Nullable Filter cellFilter,
+ SnapshotFilterImpl snapshotFilter,
+ HBaseTransaction hbaseTransaction) {
+this.userFilter = cellFilter;
+this.snapshotFilter = snapshotFilter;
+shadowCellCache = new HashMap<>();
+this.hbaseTransaction = hbaseTransaction;
+familyDeletionCache = new HashMap();
+}
+
+@Override
+public ReturnCode filterKeyValue(Cell v) throws IOException {
+if (CellUtils.isShadowCell(v)) {
+Long commitTs = Bytes.toLong(CellUtil.cloneValue(v));
+shadowCellCache.put(v.getTimestamp(), commitTs);
+// Continue getting shadow cells until one of them fits this
transaction
+if (hbaseTransaction.getStartTimestamp() >= commitTs) {
--- End diff --
Why do we keep ones that committed after the transaction timestamp?
> Implement visibility filter as pure HBase Filter
>
>
> Key: OMID-102
> URL: https://issues.apache.org/jira/browse/OMID-102
> Project: Apache Omid
> Issue Type: Sub-task
>Reporter: James Taylor
>Assignee: Yonatan Gottesman
>Priority: Major
>
> The way Omid currently filters through it's own RegionScanner won't work the
> way it's implemented (i.e. the way the filtering is done *after* the next
> call). The reason is that the state of HBase filters get messed up since
> these filters will start to see cells that it shouldn't (i.e. cells that
> would be filtered based on snapshot isolation). It cannot be worked around by
> manually running filters afterwards because filters may issue seek calls
> which are handled during the running of scans by HBase.
>
> Instead, the filtering needs to be implemented as a pure HBase filter and
> that filter needs to delegate to the other, delegate filter once it's
> determined that the cell is visible. See Tephra's TransactionVisibilityFilter
> and they way it calls the delegate filter (cellFilters) only after it's
> determined that the cell is visible. You may run into TEPHRA-169 without
> including the CellSkipFilter too.
> Because it'll be easier if you see shadow cells *before* their corresponding
> real cells you can prefix instead of suffix the column qualifiers to
>
[jira] [Commented] (OMID-102) Implement visibility filter as pure HBase Filter
[
https://issues.apache.org/jira/browse/OMID-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16565268#comment-16565268
]
ASF GitHub Bot commented on OMID-102:
-
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206866085
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
+
+public SnapshotFilter getSnapshotFilter() {
+return snapshotFilter;
+}
+
+public TransactionVisibilityFilter(@Nullable Filter cellFilter,
+ SnapshotFilterImpl snapshotFilter,
+ HBaseTransaction hbaseTransaction) {
+this.userFilter = cellFilter;
+this.snapshotFilter = snapshotFilter;
+shadowCellCache = new HashMap<>();
+this.hbaseTransaction = hbaseTransaction;
+familyDeletionCache = new HashMap();
+}
+
+@Override
+public ReturnCode filterKeyValue(Cell v) throws IOException {
+if (CellUtils.isShadowCell(v)) {
+Long commitTs = Bytes.toLong(CellUtil.cloneValue(v));
+shadowCellCache.put(v.getTimestamp(), commitTs);
+// Continue getting shadow cells until one of them fits this
transaction
+if (hbaseTransaction.getStartTimestamp() >= commitTs) {
+return ReturnCode.NEXT_COL;
+} else {
+return ReturnCode.SKIP;
+}
+} else if (CellUtils.isFamilyDeleteCell(v)) {
+//Delete is part of this transaction
+if (snapshotFilter.getTSIfInTransaction(v,
hbaseTransaction).isPresent()) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
v.getTimestamp());
+return ReturnCode.NEXT_COL;
+}
+
+if (shadowCellCache.containsKey(v.getTimestamp()) &&
+hbaseTransaction.getStartTimestamp() >=
shadowCellCache.get(v.getTimestamp())) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
shadowCellCache.get(v.getTimestamp()));
+return ReturnCode.NEXT_COL;
+}
+
+// Try to get shadow cell from region
+final Get get = new Get(CellUtil.cloneRow(v));
+get.setTimeStamp(v.getTimestamp()).setMaxVersions(1);
+get.addColumn(CellUtil.cloneFamily(v),
CellUtils.addShadowCellSuffix(CellUtils.FAMILY_DELETE_QUALIFIER));
+Result deleteFamilySC =
snapshotFilter.getTableAccessWrapper().get(get);
+
+if (!deleteFamilySC.isEmpty() &&
+
Bytes.toLong(CellUtil.cloneValue(deleteFamilySC.rawCells()[0] )) <
hbaseTransaction.getStartTimestamp()){
+
[jira] [Commented] (OMID-102) Implement visibility filter as pure HBase Filter
[
https://issues.apache.org/jira/browse/OMID-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16565266#comment-16565266
]
ASF GitHub Bot commented on OMID-102:
-
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206858386
--- Diff:
hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java
---
@@ -0,0 +1,248 @@
+/*
+ * 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 com.google.common.base.Optional;
+import com.sun.istack.Nullable;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Bytes;
+
+import java.io.IOException;
+import java.util.*;
+
+public class TransactionVisibilityFilter extends FilterBase {
+
+// optional sub-filter to apply to visible cells
+private final Filter userFilter;
+private final SnapshotFilterImpl snapshotFilter;
+private final Map shadowCellCache;
+private final HBaseTransaction hbaseTransaction;
+private final Map familyDeletionCache;
+
+public SnapshotFilter getSnapshotFilter() {
+return snapshotFilter;
+}
+
+public TransactionVisibilityFilter(@Nullable Filter cellFilter,
+ SnapshotFilterImpl snapshotFilter,
+ HBaseTransaction hbaseTransaction) {
+this.userFilter = cellFilter;
+this.snapshotFilter = snapshotFilter;
+shadowCellCache = new HashMap<>();
+this.hbaseTransaction = hbaseTransaction;
+familyDeletionCache = new HashMap();
+}
+
+@Override
+public ReturnCode filterKeyValue(Cell v) throws IOException {
+if (CellUtils.isShadowCell(v)) {
+Long commitTs = Bytes.toLong(CellUtil.cloneValue(v));
+shadowCellCache.put(v.getTimestamp(), commitTs);
+// Continue getting shadow cells until one of them fits this
transaction
+if (hbaseTransaction.getStartTimestamp() >= commitTs) {
+return ReturnCode.NEXT_COL;
+} else {
+return ReturnCode.SKIP;
+}
+} else if (CellUtils.isFamilyDeleteCell(v)) {
+//Delete is part of this transaction
+if (snapshotFilter.getTSIfInTransaction(v,
hbaseTransaction).isPresent()) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
v.getTimestamp());
+return ReturnCode.NEXT_COL;
+}
+
+if (shadowCellCache.containsKey(v.getTimestamp()) &&
+hbaseTransaction.getStartTimestamp() >=
shadowCellCache.get(v.getTimestamp())) {
+
familyDeletionCache.put(Bytes.toString(CellUtil.cloneFamily(v)),
shadowCellCache.get(v.getTimestamp()));
+return ReturnCode.NEXT_COL;
+}
+
+// Try to get shadow cell from region
+final Get get = new Get(CellUtil.cloneRow(v));
+get.setTimeStamp(v.getTimestamp()).setMaxVersions(1);
+get.addColumn(CellUtil.cloneFamily(v),
CellUtils.addShadowCellSuffix(CellUtils.FAMILY_DELETE_QUALIFIER));
+Result deleteFamilySC =
snapshotFilter.getTableAccessWrapper().get(get);
+
+if (!deleteFamilySC.isEmpty() &&
+
Bytes.toLong(CellUtil.cloneValue(deleteFamilySC.rawCells()[0] )) <
hbaseTransaction.getStartTimestamp()){
+
[jira] [Commented] (OMID-102) Implement visibility filter as pure HBase Filter
[
https://issues.apache.org/jira/browse/OMID-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16565261#comment-16565261
]
ASF GitHub Bot commented on OMID-102:
-
Github user ohadshacham commented on a diff in the pull request:
https://github.com/apache/incubator-omid/pull/41#discussion_r206843754
--- Diff:
hbase-common/src/main/java/org/apache/omid/transaction/CellUtils.java ---
@@ -382,13 +385,16 @@ public int hashCode() {
hasher.putBytes(cell.getRowArray(), cell.getRowOffset(),
cell.getRowLength());
hasher.putBytes(cell.getFamilyArray(), cell.getFamilyOffset(),
cell.getFamilyLength());
int qualifierLength = cell.getQualifierLength();
+int qualifierOffset = cell.getQualifierOffset();
if (isShadowCell()) { // Update qualifier length when
qualifier is shadow cell
qualifierLength =
qualifierLengthFromShadowCellQualifier(cell.getQualifierArray(),
cell.getQualifierOffset(),
cell.getQualifierLength());
+qualifierOffset = qualifierOffset + 1;
--- End diff --
Will it work when the shadow cell prefix is absent? legacy data.
> Implement visibility filter as pure HBase Filter
>
>
> Key: OMID-102
> URL: https://issues.apache.org/jira/browse/OMID-102
> Project: Apache Omid
> Issue Type: Sub-task
>Reporter: James Taylor
>Assignee: Yonatan Gottesman
>Priority: Major
>
> The way Omid currently filters through it's own RegionScanner won't work the
> way it's implemented (i.e. the way the filtering is done *after* the next
> call). The reason is that the state of HBase filters get messed up since
> these filters will start to see cells that it shouldn't (i.e. cells that
> would be filtered based on snapshot isolation). It cannot be worked around by
> manually running filters afterwards because filters may issue seek calls
> which are handled during the running of scans by HBase.
>
> Instead, the filtering needs to be implemented as a pure HBase filter and
> that filter needs to delegate to the other, delegate filter once it's
> determined that the cell is visible. See Tephra's TransactionVisibilityFilter
> and they way it calls the delegate filter (cellFilters) only after it's
> determined that the cell is visible. You may run into TEPHRA-169 without
> including the CellSkipFilter too.
> Because it'll be easier if you see shadow cells *before* their corresponding
> real cells you can prefix instead of suffix the column qualifiers to
> guarantee that you'd see the shadow cells prior to the actual cells. Or you
> could buffer cells in your filter prior to omitting them. Another issue would
> be if the shadow cells aren't found and you need to consult the commit table
> - I suppose if the shadow cells are first, this logic would be easier to know
> when it needs to be called.
>
> To reproduce, see the Phoenix unit tests
> FlappingTransactionIT.testInflightUpdateNotSeen() and
> testInflightDeleteNotSeen().
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OMID-102) Implement visibility filter as pure HBase Filter
[ https://issues.apache.org/jira/browse/OMID-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16565267#comment-16565267 ] ASF GitHub Bot commented on OMID-102: - Github user ohadshacham commented on a diff in the pull request: https://github.com/apache/incubator-omid/pull/41#discussion_r206849947 --- Diff: hbase-coprocessor/src/main/java/org/apache/omid/transaction/TransactionVisibilityFilter.java --- @@ -0,0 +1,248 @@ +/* + * 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 com.google.common.base.Optional; +import com.sun.istack.Nullable; --- End diff -- Different Nullable usage > Implement visibility filter as pure HBase Filter > > > Key: OMID-102 > URL: https://issues.apache.org/jira/browse/OMID-102 > Project: Apache Omid > Issue Type: Sub-task >Reporter: James Taylor >Assignee: Yonatan Gottesman >Priority: Major > > The way Omid currently filters through it's own RegionScanner won't work the > way it's implemented (i.e. the way the filtering is done *after* the next > call). The reason is that the state of HBase filters get messed up since > these filters will start to see cells that it shouldn't (i.e. cells that > would be filtered based on snapshot isolation). It cannot be worked around by > manually running filters afterwards because filters may issue seek calls > which are handled during the running of scans by HBase. > > Instead, the filtering needs to be implemented as a pure HBase filter and > that filter needs to delegate to the other, delegate filter once it's > determined that the cell is visible. See Tephra's TransactionVisibilityFilter > and they way it calls the delegate filter (cellFilters) only after it's > determined that the cell is visible. You may run into TEPHRA-169 without > including the CellSkipFilter too. > Because it'll be easier if you see shadow cells *before* their corresponding > real cells you can prefix instead of suffix the column qualifiers to > guarantee that you'd see the shadow cells prior to the actual cells. Or you > could buffer cells in your filter prior to omitting them. Another issue would > be if the shadow cells aren't found and you need to consult the commit table > - I suppose if the shadow cells are first, this logic would be easier to know > when it needs to be called. > > To reproduce, see the Phoenix unit tests > FlappingTransactionIT.testInflightUpdateNotSeen() and > testInflightDeleteNotSeen(). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OMID-102) Implement visibility filter as pure HBase Filter
[ https://issues.apache.org/jira/browse/OMID-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16565061#comment-16565061 ] ASF GitHub Bot commented on OMID-102: - GitHub user yonigottesman reopened a pull request: https://github.com/apache/incubator-omid/pull/41 [OMID-102] Support for user Filter when using coprocessor for snapshot filtering You can merge this pull request into a Git repository by running: $ git pull https://github.com/yonigottesman/incubator-omid scan_filters Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-omid/pull/41.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 #41 commit 41554ec4c3cdfdbfd271e9adbef866b3653c3382 Author: Yonatan Gottesman Date: 2018-07-25T06:40:14Z Support for user Filter when using coprocessor for snapshot filtering commit ba575384dcdca427bf80e55dcee58a1ed93bf744 Author: Yonatan Gottesman Date: 2018-07-25T07:41:42Z In coprocessor filtering, get shadow cell of delete family before going to commit table commit 26469a0893b4c9bc36e68d66583951fb6e82cf9d Author: Yonatan Gottesman Date: 2018-07-26T08:15:27Z add inMemoryCommitTable client option in omid coprocessor for testing commit 8d632670ab580ca04e0c5cec555211d1fa71a11a Author: Yonatan Gottesman Date: 2018-07-31T11:29:13Z Fix visibilityFilter to check if delete family is in current TX commit b2980c7cea2ef9c83a772d4ce3bd9a82f5ae4d81 Author: Yonatan Gottesman Date: 2018-08-01T09:34:00Z Merge OMID-105 changes > Implement visibility filter as pure HBase Filter > > > Key: OMID-102 > URL: https://issues.apache.org/jira/browse/OMID-102 > Project: Apache Omid > Issue Type: Sub-task >Reporter: James Taylor >Assignee: Yonatan Gottesman >Priority: Major > > The way Omid currently filters through it's own RegionScanner won't work the > way it's implemented (i.e. the way the filtering is done *after* the next > call). The reason is that the state of HBase filters get messed up since > these filters will start to see cells that it shouldn't (i.e. cells that > would be filtered based on snapshot isolation). It cannot be worked around by > manually running filters afterwards because filters may issue seek calls > which are handled during the running of scans by HBase. > > Instead, the filtering needs to be implemented as a pure HBase filter and > that filter needs to delegate to the other, delegate filter once it's > determined that the cell is visible. See Tephra's TransactionVisibilityFilter > and they way it calls the delegate filter (cellFilters) only after it's > determined that the cell is visible. You may run into TEPHRA-169 without > including the CellSkipFilter too. > Because it'll be easier if you see shadow cells *before* their corresponding > real cells you can prefix instead of suffix the column qualifiers to > guarantee that you'd see the shadow cells prior to the actual cells. Or you > could buffer cells in your filter prior to omitting them. Another issue would > be if the shadow cells aren't found and you need to consult the commit table > - I suppose if the shadow cells are first, this logic would be easier to know > when it needs to be called. > > To reproduce, see the Phoenix unit tests > FlappingTransactionIT.testInflightUpdateNotSeen() and > testInflightDeleteNotSeen(). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OMID-102) Implement visibility filter as pure HBase Filter
[ https://issues.apache.org/jira/browse/OMID-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16565060#comment-16565060 ] ASF GitHub Bot commented on OMID-102: - Github user yonigottesman closed the pull request at: https://github.com/apache/incubator-omid/pull/41 > Implement visibility filter as pure HBase Filter > > > Key: OMID-102 > URL: https://issues.apache.org/jira/browse/OMID-102 > Project: Apache Omid > Issue Type: Sub-task >Reporter: James Taylor >Assignee: Yonatan Gottesman >Priority: Major > > The way Omid currently filters through it's own RegionScanner won't work the > way it's implemented (i.e. the way the filtering is done *after* the next > call). The reason is that the state of HBase filters get messed up since > these filters will start to see cells that it shouldn't (i.e. cells that > would be filtered based on snapshot isolation). It cannot be worked around by > manually running filters afterwards because filters may issue seek calls > which are handled during the running of scans by HBase. > > Instead, the filtering needs to be implemented as a pure HBase filter and > that filter needs to delegate to the other, delegate filter once it's > determined that the cell is visible. See Tephra's TransactionVisibilityFilter > and they way it calls the delegate filter (cellFilters) only after it's > determined that the cell is visible. You may run into TEPHRA-169 without > including the CellSkipFilter too. > Because it'll be easier if you see shadow cells *before* their corresponding > real cells you can prefix instead of suffix the column qualifiers to > guarantee that you'd see the shadow cells prior to the actual cells. Or you > could buffer cells in your filter prior to omitting them. Another issue would > be if the shadow cells aren't found and you need to consult the commit table > - I suppose if the shadow cells are first, this logic would be easier to know > when it needs to be called. > > To reproduce, see the Phoenix unit tests > FlappingTransactionIT.testInflightUpdateNotSeen() and > testInflightDeleteNotSeen(). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OMID-106) Delete does not use write timestamp after checkpoint
[ https://issues.apache.org/jira/browse/OMID-106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16564988#comment-16564988 ] ASF GitHub Bot commented on OMID-106: - Github user ohadshacham closed the pull request at: https://github.com/apache/incubator-omid/pull/40 > Delete does not use write timestamp after checkpoint > > > Key: OMID-106 > URL: https://issues.apache.org/jira/browse/OMID-106 > Project: Apache Omid > Issue Type: Sub-task >Reporter: Ohad Shacham >Assignee: Ohad Shacham >Priority: Major > > Delete should use write timestamp when writing family deletion marker. This > is noticeable after a checkpoint. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OMID-105) Family deletion seek should continue when a tentative value is found
[ https://issues.apache.org/jira/browse/OMID-105?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16564987#comment-16564987 ] ASF GitHub Bot commented on OMID-105: - Github user ohadshacham closed the pull request at: https://github.com/apache/incubator-omid/pull/39 > Family deletion seek should continue when a tentative value is found > - > > Key: OMID-105 > URL: https://issues.apache.org/jira/browse/OMID-105 > Project: Apache Omid > Issue Type: Sub-task >Reporter: Ohad Shacham >Assignee: Ohad Shacham >Priority: Major > > > When a tentative family deletion marker is found. We need to continue looking > until we either find a committed one in the past or no committed family > deletion marker for this column is found. Otherwise, we might miss committed > family deletion markers that exists in a transaction snapshot. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
