[GitHub] [hbase] wchevreuil commented on a change in pull request #837: HBASE-23309: Adding the flexibility to ChainWalEntryFilter to filter the whole entry if all cells get filtered
wchevreuil commented on a change in pull request #837: HBASE-23309: Adding the
flexibility to ChainWalEntryFilter to filter the whole entry if all cells get
filtered
URL: https://github.com/apache/hbase/pull/837#discussion_r352084573
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/CustomChainWALEntryFilter.java
##
@@ -0,0 +1,68 @@
+/**
+ * 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.replication;
+
+import org.apache.hadoop.hbase.HBaseInterfaceAudience;
+import org.apache.hadoop.hbase.wal.WAL;
+import
org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import java.util.List;
+
+/**
+ * A {@link ChainWALEntryFilter} for providing more flexible options
+ */
+@InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.REPLICATION)
+public class CustomChainWALEntryFilter extends ChainWALEntryFilter {
+
+ private boolean filterEmptyEntry = false;
+
+ public CustomChainWALEntryFilter(final WALEntryFilter... filters) {
+super(filters);
+ }
+
+ public CustomChainWALEntryFilter(final List filters) {
+super(filters);
+ }
+
+ @Override
+ public WAL.Entry filter(WAL.Entry entry) {
+filterEntry(entry);
+if (entry == null) {
+ return null;
+}
+
+filterCells(entry);
Review comment:
I guess we could user super method for this block?
`super.filter(entry);`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] wchevreuil commented on a change in pull request #837: HBASE-23309: Adding the flexibility to ChainWalEntryFilter to filter the whole entry if all cells get filtered
wchevreuil commented on a change in pull request #837: HBASE-23309: Adding the
flexibility to ChainWalEntryFilter to filter the whole entry if all cells get
filtered
URL: https://github.com/apache/hbase/pull/837#discussion_r352088255
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/CustomChainWALEntryFilter.java
##
@@ -0,0 +1,68 @@
+/**
+ * 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.replication;
+
+import org.apache.hadoop.hbase.HBaseInterfaceAudience;
+import org.apache.hadoop.hbase.wal.WAL;
+import
org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import java.util.List;
+
+/**
+ * A {@link ChainWALEntryFilter} for providing more flexible options
+ */
+@InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.REPLICATION)
+public class CustomChainWALEntryFilter extends ChainWALEntryFilter {
Review comment:
`CustomChainWALEntryFilter` name is not very intuitive for a built-in
filter. Maybe something like `ChainWALEmptyEntryFilter` or
`ChainWALNullEntryFilter`?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] wchevreuil commented on a change in pull request #837: HBASE-23309: Adding the flexibility to ChainWalEntryFilter to filter the whole entry if all cells get filtered
wchevreuil commented on a change in pull request #837: HBASE-23309: Adding the flexibility to ChainWalEntryFilter to filter the whole entry if all cells get filtered URL: https://github.com/apache/hbase/pull/837#discussion_r347898189 ## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/BaseReplicationEndpoint.java ## @@ -41,15 +41,16 @@ public static final String REPLICATION_WALENTRYFILTER_CONFIG_KEY = "hbase.replication.source.custom.walentryfilters"; protected Context ctx; + private ReplicationPeer replicationPeer; Review comment: Why this needs to be a global variable? Seems to be used only on _init_ method. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] wchevreuil commented on a change in pull request #837: HBASE-23309: Adding the flexibility to ChainWalEntryFilter to filter the whole entry if all cells get filtered
wchevreuil commented on a change in pull request #837: HBASE-23309: Adding the
flexibility to ChainWalEntryFilter to filter the whole entry if all cells get
filtered
URL: https://github.com/apache/hbase/pull/837#discussion_r347990202
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/ChainWALEntryFilter.java
##
@@ -68,13 +82,17 @@ public void initCellFilters() {
@Override
public Entry filter(Entry entry) {
+
for (WALEntryFilter filter : filters) {
if (entry == null) {
return null;
}
entry = filter.filter(entry);
}
filterCells(entry);
+if (shouldFilterEmptyEntry() && entry != null &&
entry.getEdit().isEmpty()) {
Review comment:
I think this additional logic would fit better in a built-in filter (one of
@apurtell suggestions above). That would not need the extra config property nor
changes in _BaseRepplicationEndpoint_
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] wchevreuil commented on a change in pull request #837: HBASE-23309: Adding the flexibility to ChainWalEntryFilter to filter the whole entry if all cells get filtered
wchevreuil commented on a change in pull request #837: HBASE-23309: Adding the
flexibility to ChainWalEntryFilter to filter the whole entry if all cells get
filtered
URL: https://github.com/apache/hbase/pull/837#discussion_r347943456
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/ChainWALEntryFilter.java
##
@@ -68,13 +82,17 @@ public void initCellFilters() {
@Override
public Entry filter(Entry entry) {
+
Review comment:
nit: remove unnecessary empty line.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
