You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2019/04/02 01:54:44 UTC
[incubator-pinot] 01/01: Fix the race condition for real-time
inverted index reader
This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch fix_realtime_race_condition
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
commit 430d04c14b77cf189cdacbf90e93ba11477c6a3c
Author: Jackie (Xiaotian) Jiang <xa...@linkedin.com>
AuthorDate: Mon Apr 1 18:51:59 2019 -0700
Fix the race condition for real-time inverted index reader
When getting doc Ids from RealtimeInvertedIndexReader, the given
dict Id might not be added to the inverted index yet. We first add
the value to the dictionary. Before the value is added to the
inverted index, the query might have a predicate which matches the
newly added value. In that case, the given dictionary Id does not
exist in the inverted index, and we return an empty bitmap.
Simplify BitmapBasedFilterOperator because getDocIds() never
returns null.
Add a test for the change.
---
.../operator/filter/BitmapBasedFilterOperator.java | 23 ++-----
.../invertedindex/RealtimeInvertedIndexReader.java | 7 ++
.../RealtimeInvertedIndexReaderTest.java | 78 ++++++++++++++++++++++
3 files changed, 89 insertions(+), 19 deletions(-)
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BitmapBasedFilterOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BitmapBasedFilterOperator.java
index 64d7c3e..7a82c15 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BitmapBasedFilterOperator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BitmapBasedFilterOperator.java
@@ -19,8 +19,6 @@
package org.apache.pinot.core.operator.filter;
import com.google.common.base.Preconditions;
-import java.util.ArrayList;
-import java.util.List;
import org.apache.pinot.core.common.DataSource;
import org.apache.pinot.core.operator.blocks.FilterBlock;
import org.apache.pinot.core.operator.docidsets.BitmapDocIdSet;
@@ -78,27 +76,14 @@ public class BitmapBasedFilterOperator extends BaseFilterOperator {
int[] dictIds = _exclusive ? _predicateEvaluator.getNonMatchingDictIds() : _predicateEvaluator.getMatchingDictIds();
- // For realtime use case, it is possible that inverted index has not yet generated for the given dict id, so we
- // filter out null bitmaps
InvertedIndexReader invertedIndex = _dataSource.getInvertedIndex();
int length = dictIds.length;
- List<ImmutableRoaringBitmap> bitmaps = new ArrayList<>(length);
- for (int dictId : dictIds) {
- ImmutableRoaringBitmap bitmap = (ImmutableRoaringBitmap) invertedIndex.getDocIds(dictId);
- if (bitmap != null) {
- bitmaps.add(bitmap);
- }
+ ImmutableRoaringBitmap[] bitmaps = new ImmutableRoaringBitmap[length];
+ for (int i = 0; i < length; i++) {
+ bitmaps[i] = (ImmutableRoaringBitmap) invertedIndex.getDocIds(dictIds[i]);
}
- // Log size diff to verify the fix
- int numBitmaps = bitmaps.size();
- if (numBitmaps != length) {
- LOGGER.info("Not all inverted indexes are generated, numDictIds: {}, numBitmaps: {}", length, numBitmaps);
- }
-
- return new FilterBlock(
- new BitmapDocIdSet(bitmaps.toArray(new ImmutableRoaringBitmap[numBitmaps]), _startDocId, _endDocId,
- _exclusive));
+ return new FilterBlock(new BitmapDocIdSet(bitmaps, _startDocId, _endDocId, _exclusive));
}
@Override
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/realtime/impl/invertedindex/RealtimeInvertedIndexReader.java b/pinot-core/src/main/java/org/apache/pinot/core/realtime/impl/invertedindex/RealtimeInvertedIndexReader.java
index 363f688..1250da0 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/realtime/impl/invertedindex/RealtimeInvertedIndexReader.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/realtime/impl/invertedindex/RealtimeInvertedIndexReader.java
@@ -60,6 +60,13 @@ public class RealtimeInvertedIndexReader implements InvertedIndexReader<MutableR
ThreadSafeMutableRoaringBitmap bitmap;
try {
_readLock.lock();
+ // NOTE: the given dict Id might not be added to the inverted index yet. We first add the value to the dictionary.
+ // Before the value is added to the inverted index, the query might have a predicate which matches the newly added
+ // value. In that case, the given dictionary Id does not exist in the inverted index, and we return an empty
+ // bitmap.
+ if (_bitmaps.size() == dictId) {
+ return new MutableRoaringBitmap();
+ }
bitmap = _bitmaps.get(dictId);
} finally {
_readLock.unlock();
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/realtime/impl/invertedindex/RealtimeInvertedIndexReaderTest.java b/pinot-core/src/test/java/org/apache/pinot/core/realtime/impl/invertedindex/RealtimeInvertedIndexReaderTest.java
new file mode 100644
index 0000000..2191a23
--- /dev/null
+++ b/pinot-core/src/test/java/org/apache/pinot/core/realtime/impl/invertedindex/RealtimeInvertedIndexReaderTest.java
@@ -0,0 +1,78 @@
+/**
+ * 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.pinot.core.realtime.impl.invertedindex;
+
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+
+
+public class RealtimeInvertedIndexReaderTest {
+
+ @Test
+ public void testRealtimeInvertedIndexReader() {
+ RealtimeInvertedIndexReader realtimeInvertedIndexReader = new RealtimeInvertedIndexReader();
+
+ // Dict Id not added yet
+ MutableRoaringBitmap docIds = realtimeInvertedIndexReader.getDocIds(0);
+ assertNotNull(docIds);
+ assertTrue(docIds.isEmpty());
+
+ // Add dict Id 0, doc Id 0 to the inverted index
+ realtimeInvertedIndexReader.add(0, 0);
+ docIds = realtimeInvertedIndexReader.getDocIds(0);
+ assertNotNull(docIds);
+ assertFalse(docIds.isEmpty());
+ assertTrue(docIds.contains(0));
+ assertFalse(docIds.contains(1));
+ docIds = realtimeInvertedIndexReader.getDocIds(1);
+ assertNotNull(docIds);
+ assertTrue(docIds.isEmpty());
+
+ // Add dict Id 0, doc Id 1 to the inverted index
+ realtimeInvertedIndexReader.add(0, 1);
+ docIds = realtimeInvertedIndexReader.getDocIds(0);
+ assertNotNull(docIds);
+ assertFalse(docIds.isEmpty());
+ assertTrue(docIds.contains(0));
+ assertTrue(docIds.contains(1));
+ docIds = realtimeInvertedIndexReader.getDocIds(1);
+ assertNotNull(docIds);
+ assertTrue(docIds.isEmpty());
+
+ // Add dict Id 1, doc Id 1 to the inverted index
+ realtimeInvertedIndexReader.add(1, 1);
+ docIds = realtimeInvertedIndexReader.getDocIds(0);
+ assertNotNull(docIds);
+ assertFalse(docIds.isEmpty());
+ assertTrue(docIds.contains(0));
+ assertTrue(docIds.contains(1));
+ docIds = realtimeInvertedIndexReader.getDocIds(1);
+ assertNotNull(docIds);
+ assertFalse(docIds.isEmpty());
+ assertFalse(docIds.contains(0));
+ assertTrue(docIds.contains(1));
+ docIds = realtimeInvertedIndexReader.getDocIds(2);
+ assertNotNull(docIds);
+ assertTrue(docIds.isEmpty());
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org