You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/11/15 08:35:24 UTC
[GitHub] [pinot] KKcorps opened a new pull request, #9802: Add memomry optimized dimension table
KKcorps opened a new pull request, #9802:
URL: https://github.com/apache/pinot/pull/9802
Currently, we store the whole row in the Dimension table hash map for fast lookups.
However, in some cases, we can trade off the speed for memory efficiency.
In this PR, I am adding a new implementation that only stores the segment reference and docID, and the actual value is read at the time of lookup.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] saurabhd336 commented on a diff in pull request #9802: Add memory optimized dimension table
Posted by "saurabhd336 (via GitHub)" <gi...@apache.org>.
saurabhd336 commented on code in PR #9802:
URL: https://github.com/apache/pinot/pull/9802#discussion_r1151805078
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/MemoryOptimizedDimensionTable.java:
##########
@@ -0,0 +1,93 @@
+/**
+ * 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.data.manager.offline;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.segment.local.data.manager.SegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.data.readers.PrimaryKey;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+class MemoryOptimizedDimensionTable implements DimensionTable {
+ private static final Logger LOGGER = LoggerFactory.getLogger(MemoryOptimizedDimensionTable.class);
+
+ private final Map<PrimaryKey, LookupRecordLocation> _lookupTable;
+ private final Schema _tableSchema;
+ private final List<String> _primaryKeyColumns;
+ private final GenericRow _reuse = new GenericRow();
+ private final List<SegmentDataManager> _segmentDataManagers;
+ private final TableDataManager _tableDataManager;
+
+ MemoryOptimizedDimensionTable(Schema tableSchema, List<String> primaryKeyColumns,
+ Map<PrimaryKey, LookupRecordLocation> lookupTable, List<SegmentDataManager> segmentDataManagers,
+ TableDataManager tableDataManager) {
+ _tableSchema = tableSchema;
+ _primaryKeyColumns = primaryKeyColumns;
+ _lookupTable = lookupTable;
+ _segmentDataManagers = segmentDataManagers;
+ _tableDataManager = tableDataManager;
+ }
+
+ @Override
+ public List<String> getPrimaryKeyColumns() {
+ return _primaryKeyColumns;
+ }
+
+ @Override
+ public GenericRow get(PrimaryKey pk) {
+ LookupRecordLocation lookupRecordLocation = _lookupTable.get(pk);
+ if (lookupRecordLocation == null) {
+ return null;
+ }
+ return lookupRecordLocation.getRecord(_reuse);
Review Comment:
get can be called concurrently. Think this should be `ThreadLocal<GenericRow> _reuse` instead
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9802: Add memory optimized dimension table
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9802:
URL: https://github.com/apache/pinot/pull/9802#discussion_r1028616845
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/LookupRecordLocation.java:
##########
@@ -0,0 +1,40 @@
+/**
+ * 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.data.manager.offline;
+
+import org.apache.pinot.segment.spi.IndexSegment;
+
+
+public class LookupRecordLocation {
+ private final IndexSegment _indexSegment;
Review Comment:
We might want to cache the `PinotSegmentRecordReader` here to avoid creating it per record lookup. It shouldn't be too expensive because lookup table should be very small.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9802: Add memory optimized dimension table
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9802:
URL: https://github.com/apache/pinot/pull/9802#discussion_r1034146942
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/MemoryOptimizedDimensionTable.java:
##########
@@ -0,0 +1,71 @@
+/**
+ * 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.data.manager.offline;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.data.readers.PrimaryKey;
+
+
+class MemoryOptimizedDimensionTable implements DimensionTable {
+
+ private final Map<PrimaryKey, LookupRecordLocation> _lookupTable;
+ private final Schema _tableSchema;
+ private final List<String> _primaryKeyColumns;
+ private final GenericRow _reuse = new GenericRow();
+
+ MemoryOptimizedDimensionTable(Schema tableSchema, List<String> primaryKeyColumns) {
Review Comment:
^^
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -117,6 +134,27 @@ public void removeSegment(String segmentName) {
}
}
+ @Override
+ protected void doShutdown() {
+ closeDimensionTable(_dimensionTable, true);
Review Comment:
We shouldn't release the segments here when it is preloaded.
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -117,6 +134,27 @@ public void removeSegment(String segmentName) {
}
}
+ @Override
+ protected void doShutdown() {
+ closeDimensionTable(_dimensionTable, true);
+ }
+
+ private void closeDimensionTable(DimensionTable dimensionTable, boolean releaseSegments) {
+ if (dimensionTable != null) {
+ try {
+ dimensionTable.close();
+ if (releaseSegments) {
+ List<SegmentDataManager> segmentDataManagers = acquireAllSegments();
+ for (SegmentDataManager segmentDataManager: segmentDataManagers) {
+ releaseSegment(segmentDataManager);
+ }
Review Comment:
This is incorrect. Acquiring then release is no-op, and the segments won't be released in the end.
Instead, when preload is disabled, we can track the `SegmentDataManagers` and release them here.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #9802: Add memory optimized dimension table
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9802:
URL: https://github.com/apache/pinot/pull/9802#issuecomment-1315012378
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9802?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#9802](https://codecov.io/gh/apache/pinot/pull/9802?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0ebb0f5) into [master](https://codecov.io/gh/apache/pinot/commit/cd1a017debf95e088881b191319c521b2a3a4296?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cd1a017) will **increase** coverage by `3.63%`.
> The diff coverage is `51.25%`.
```diff
@@ Coverage Diff @@
## master #9802 +/- ##
============================================
+ Coverage 64.11% 67.75% +3.63%
+ Complexity 5385 5225 -160
============================================
Files 1903 1461 -442
Lines 102554 76348 -26206
Branches 15604 12139 -3465
============================================
- Hits 65753 51729 -14024
+ Misses 32017 20941 -11076
+ Partials 4784 3678 -1106
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `67.75% <51.25%> (-0.02%)` | :arrow_down: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9802?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...data/manager/offline/FastLookupDimensionTable.java](https://codecov.io/gh/apache/pinot/pull/9802/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvb2ZmbGluZS9GYXN0TG9va3VwRGltZW5zaW9uVGFibGUuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...e/pinot/spi/config/table/DimensionTableConfig.java](https://codecov.io/gh/apache/pinot/pull/9802/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0RpbWVuc2lvblRhYmxlQ29uZmlnLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...org/apache/pinot/spi/config/table/TableConfig.java](https://codecov.io/gh/apache/pinot/pull/9802/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlQ29uZmlnLmphdmE=) | `62.50% <33.33%> (-1.03%)` | :arrow_down: |
| [...ata/manager/offline/DimensionTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/9802/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvb2ZmbGluZS9EaW1lbnNpb25UYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `53.00% <55.26%> (-24.78%)` | :arrow_down: |
| [...manager/offline/MemoryOptimizedDimensionTable.java](https://codecov.io/gh/apache/pinot/pull/9802/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvb2ZmbGluZS9NZW1vcnlPcHRpbWl6ZWREaW1lbnNpb25UYWJsZS5qYXZh) | `81.25% <81.25%> (ø)` | |
| [...ore/data/manager/offline/LookupRecordLocation.java](https://codecov.io/gh/apache/pinot/pull/9802/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvb2ZmbGluZS9Mb29rdXBSZWNvcmRMb2NhdGlvbi5qYXZh) | `100.00% <100.00%> (ø)` | |
| [...pache/pinot/core/transport/AsyncQueryResponse.java](https://codecov.io/gh/apache/pinot/pull/9802/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS90cmFuc3BvcnQvQXN5bmNRdWVyeVJlc3BvbnNlLmphdmE=) | `74.57% <0.00%> (-3.43%)` | :arrow_down: |
| [...ot/query/runtime/operator/MailboxSendOperator.java](https://codecov.io/gh/apache/pinot/pull/9802/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9NYWlsYm94U2VuZE9wZXJhdG9yLmphdmE=) | `86.04% <0.00%> (-2.50%)` | :arrow_down: |
| [...a/org/apache/pinot/core/transport/QueryRouter.java](https://codecov.io/gh/apache/pinot/pull/9802/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS90cmFuc3BvcnQvUXVlcnlSb3V0ZXIuamF2YQ==) | `75.00% <0.00%> (-1.67%)` | :arrow_down: |
| ... and [471 more](https://codecov.io/gh/apache/pinot/pull/9802/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9802: Add memory optimized dimension table
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9802:
URL: https://github.com/apache/pinot/pull/9802#discussion_r1036358541
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -117,6 +135,26 @@ public void removeSegment(String segmentName) {
}
}
+ @Override
+ protected void doShutdown() {
+ closeDimensionTable(_dimensionTable, _disablePreload);
+ }
+
+ private void closeDimensionTable(DimensionTable dimensionTable, boolean releaseSegments) {
+ if (dimensionTable != null) {
Review Comment:
(minor) This check is redundant
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -117,6 +135,26 @@ public void removeSegment(String segmentName) {
}
}
+ @Override
+ protected void doShutdown() {
+ closeDimensionTable(_dimensionTable, _disablePreload);
+ }
+
+ private void closeDimensionTable(DimensionTable dimensionTable, boolean releaseSegments) {
+ if (dimensionTable != null) {
+ try {
+ dimensionTable.close();
+ if (releaseSegments && _segmentDataManagers != null) {
+ for (SegmentDataManager segmentDataManager: _segmentDataManagers) {
Review Comment:
This doesn't work because the current `_segmentDataManagers` are for the new dimension table, and here we are closing the already replaced dimension table.
We can pass the acquired `List<SegmentDataManager>` into the `MemoryOptimizedDimensionTable`, and close it within the `MemoryOptimizedDimensionTable.close()`. Basically the dimension table implementation knows whether it needs to release the segments, instead of the caller
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] KKcorps commented on a diff in pull request #9802: Add memory optimized dimension table
Posted by GitBox <gi...@apache.org>.
KKcorps commented on code in PR #9802:
URL: https://github.com/apache/pinot/pull/9802#discussion_r1032761596
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -156,7 +178,43 @@ private DimensionTable createDimensionTable() {
}
}
}
- return new DimensionTable(schema, primaryKeyColumns, lookupTable);
+ return new FastLookupDimensionTable(schema, primaryKeyColumns, lookupTable);
+ } finally {
+ for (SegmentDataManager segmentManager : segmentManagers) {
+ releaseSegment(segmentManager);
+ }
+ }
+ }
+
+ private DimensionTable createMemOptimisedDimensionTable() {
+ Schema schema = ZKMetadataProvider.getTableSchema(_propertyStore, _tableNameWithType);
+ Preconditions.checkState(schema != null, "Failed to find schema for dimension table: %s", _tableNameWithType);
+
+ List<String> primaryKeyColumns = schema.getPrimaryKeyColumns();
+ Preconditions.checkState(CollectionUtils.isNotEmpty(primaryKeyColumns),
+ "Primary key columns must be configured for dimension table: %s", _tableNameWithType);
+
+ Map<PrimaryKey, LookupRecordLocation> lookupTable = new HashMap<>();
+ List<SegmentDataManager> segmentManagers = acquireAllSegments();
+ try {
+ for (SegmentDataManager segmentManager : segmentManagers) {
+ IndexSegment indexSegment = segmentManager.getSegment();
+ int numTotalDocs = indexSegment.getSegmentMetadata().getTotalDocs();
+ if (numTotalDocs > 0) {
+ try (PinotSegmentRecordReader recordReader = new PinotSegmentRecordReader()) {
Review Comment:
My bad. I have changed the logic now to close the segments only when shutting down the table data manager. The record reader is closed when we replace the table we newer instance. Both of these happen only when preload is disabled, otherwise old logic is applied.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9802: Add memory optimized dimension table
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9802:
URL: https://github.com/apache/pinot/pull/9802#discussion_r1028546083
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTable.java:
##########
@@ -18,44 +18,21 @@
*/
package org.apache.pinot.core.data.manager.offline;
-import java.util.HashMap;
import java.util.List;
-import java.util.Map;
import org.apache.pinot.spi.data.FieldSpec;
-import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.data.readers.GenericRow;
import org.apache.pinot.spi.data.readers.PrimaryKey;
-class DimensionTable {
+public interface DimensionTable<T> {
- private final Map<PrimaryKey, GenericRow> _lookupTable;
- private final Schema _tableSchema;
- private final List<String> _primaryKeyColumns;
+ List<String> getPrimaryKeyColumns();
- DimensionTable(Schema tableSchema, List<String> primaryKeyColumns) {
- this(tableSchema, primaryKeyColumns, new HashMap<>());
- }
+ GenericRow get(PrimaryKey pk);
- DimensionTable(Schema tableSchema, List<String> primaryKeyColumns, Map<PrimaryKey, GenericRow> lookupTable) {
- _lookupTable = lookupTable;
- _tableSchema = tableSchema;
- _primaryKeyColumns = primaryKeyColumns;
- }
+ boolean put(PrimaryKey pk, T value);
Review Comment:
It should be immutable. Let's remove this method, and we can also remove the generic type `T`
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -78,16 +80,31 @@ public static DimensionTableDataManager getInstanceByTableName(String tableNameW
AtomicReferenceFieldUpdater.newUpdater(DimensionTableDataManager.class, DimensionTable.class, "_dimensionTable");
private volatile DimensionTable _dimensionTable;
+ private boolean _isMemoryOptimized = true;
Review Comment:
Let's call it `_disablePreload` and default to `false` (for backward compatible)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9802: Add memory optimized dimension table
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9802:
URL: https://github.com/apache/pinot/pull/9802#discussion_r1029878916
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/MemoryOptimizedDimensionTable.java:
##########
@@ -0,0 +1,71 @@
+/**
+ * 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.data.manager.offline;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.data.readers.PrimaryKey;
+
+
+class MemoryOptimizedDimensionTable implements DimensionTable {
+
+ private final Map<PrimaryKey, LookupRecordLocation> _lookupTable;
+ private final Schema _tableSchema;
+ private final List<String> _primaryKeyColumns;
+ private final GenericRow _reuse = new GenericRow();
+
+ MemoryOptimizedDimensionTable(Schema tableSchema, List<String> primaryKeyColumns) {
Review Comment:
(minor) Do we need this constructor?
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -125,13 +142,18 @@ private void loadLookupTable() {
DimensionTable replacement;
do {
snapshot = _dimensionTable;
- replacement = createDimensionTable();
+ if (_disablePreload) {
+ replacement = createMemOptimisedDimensionTable();
+ } else {
+ replacement = createDimensionTable();
+ }
} while (!UPDATER.compareAndSet(this, snapshot, replacement));
}
private DimensionTable createDimensionTable() {
Review Comment:
(minor) Rename this method
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -156,7 +178,43 @@ private DimensionTable createDimensionTable() {
}
}
}
- return new DimensionTable(schema, primaryKeyColumns, lookupTable);
+ return new FastLookupDimensionTable(schema, primaryKeyColumns, lookupTable);
Review Comment:
(minor) Reformat
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -156,7 +178,43 @@ private DimensionTable createDimensionTable() {
}
}
}
- return new DimensionTable(schema, primaryKeyColumns, lookupTable);
+ return new FastLookupDimensionTable(schema, primaryKeyColumns, lookupTable);
+ } finally {
+ for (SegmentDataManager segmentManager : segmentManagers) {
+ releaseSegment(segmentManager);
+ }
+ }
+ }
+
+ private DimensionTable createMemOptimisedDimensionTable() {
+ Schema schema = ZKMetadataProvider.getTableSchema(_propertyStore, _tableNameWithType);
+ Preconditions.checkState(schema != null, "Failed to find schema for dimension table: %s", _tableNameWithType);
+
+ List<String> primaryKeyColumns = schema.getPrimaryKeyColumns();
+ Preconditions.checkState(CollectionUtils.isNotEmpty(primaryKeyColumns),
+ "Primary key columns must be configured for dimension table: %s", _tableNameWithType);
+
+ Map<PrimaryKey, LookupRecordLocation> lookupTable = new HashMap<>();
+ List<SegmentDataManager> segmentManagers = acquireAllSegments();
+ try {
+ for (SegmentDataManager segmentManager : segmentManagers) {
+ IndexSegment indexSegment = segmentManager.getSegment();
+ int numTotalDocs = indexSegment.getSegmentMetadata().getTotalDocs();
+ if (numTotalDocs > 0) {
+ try (PinotSegmentRecordReader recordReader = new PinotSegmentRecordReader()) {
Review Comment:
(MAJOR) This will close the record reader, which makes the record not readable from the record location. We need to figure out a way to close the record reader though (probably add a `close()` in `DimensionTable`)
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -156,7 +178,43 @@ private DimensionTable createDimensionTable() {
}
}
}
- return new DimensionTable(schema, primaryKeyColumns, lookupTable);
+ return new FastLookupDimensionTable(schema, primaryKeyColumns, lookupTable);
+ } finally {
+ for (SegmentDataManager segmentManager : segmentManagers) {
+ releaseSegment(segmentManager);
+ }
+ }
+ }
+
+ private DimensionTable createMemOptimisedDimensionTable() {
+ Schema schema = ZKMetadataProvider.getTableSchema(_propertyStore, _tableNameWithType);
+ Preconditions.checkState(schema != null, "Failed to find schema for dimension table: %s", _tableNameWithType);
+
+ List<String> primaryKeyColumns = schema.getPrimaryKeyColumns();
+ Preconditions.checkState(CollectionUtils.isNotEmpty(primaryKeyColumns),
+ "Primary key columns must be configured for dimension table: %s", _tableNameWithType);
+
+ Map<PrimaryKey, LookupRecordLocation> lookupTable = new HashMap<>();
+ List<SegmentDataManager> segmentManagers = acquireAllSegments();
+ try {
+ for (SegmentDataManager segmentManager : segmentManagers) {
+ IndexSegment indexSegment = segmentManager.getSegment();
+ int numTotalDocs = indexSegment.getSegmentMetadata().getTotalDocs();
+ if (numTotalDocs > 0) {
+ try (PinotSegmentRecordReader recordReader = new PinotSegmentRecordReader()) {
Review Comment:
Since we need to access the segment within the `DimensionTable`, there is another corner case we need to handle. Currently when a segment is removed, we release the segment before reloading the lookup table. This will cause on-the-fly query to fail.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9802: Add memory optimized dimension table
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9802:
URL: https://github.com/apache/pinot/pull/9802#discussion_r1152600051
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/MemoryOptimizedDimensionTable.java:
##########
@@ -0,0 +1,93 @@
+/**
+ * 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.data.manager.offline;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.segment.local.data.manager.SegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.data.readers.PrimaryKey;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+class MemoryOptimizedDimensionTable implements DimensionTable {
+ private static final Logger LOGGER = LoggerFactory.getLogger(MemoryOptimizedDimensionTable.class);
+
+ private final Map<PrimaryKey, LookupRecordLocation> _lookupTable;
+ private final Schema _tableSchema;
+ private final List<String> _primaryKeyColumns;
+ private final GenericRow _reuse = new GenericRow();
+ private final List<SegmentDataManager> _segmentDataManagers;
+ private final TableDataManager _tableDataManager;
+
+ MemoryOptimizedDimensionTable(Schema tableSchema, List<String> primaryKeyColumns,
+ Map<PrimaryKey, LookupRecordLocation> lookupTable, List<SegmentDataManager> segmentDataManagers,
+ TableDataManager tableDataManager) {
+ _tableSchema = tableSchema;
+ _primaryKeyColumns = primaryKeyColumns;
+ _lookupTable = lookupTable;
+ _segmentDataManagers = segmentDataManagers;
+ _tableDataManager = tableDataManager;
+ }
+
+ @Override
+ public List<String> getPrimaryKeyColumns() {
+ return _primaryKeyColumns;
+ }
+
+ @Override
+ public GenericRow get(PrimaryKey pk) {
+ LookupRecordLocation lookupRecordLocation = _lookupTable.get(pk);
+ if (lookupRecordLocation == null) {
+ return null;
+ }
+ return lookupRecordLocation.getRecord(_reuse);
Review Comment:
Good catch! I think this is a bug
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] KKcorps commented on a diff in pull request #9802: Add memory optimized dimension table
Posted by "KKcorps (via GitHub)" <gi...@apache.org>.
KKcorps commented on code in PR #9802:
URL: https://github.com/apache/pinot/pull/9802#discussion_r1151773032
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/MemoryOptimizedDimensionTable.java:
##########
@@ -0,0 +1,93 @@
+/**
+ * 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.data.manager.offline;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.segment.local.data.manager.SegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.data.readers.PrimaryKey;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+class MemoryOptimizedDimensionTable implements DimensionTable {
+ private static final Logger LOGGER = LoggerFactory.getLogger(MemoryOptimizedDimensionTable.class);
+
+ private final Map<PrimaryKey, LookupRecordLocation> _lookupTable;
+ private final Schema _tableSchema;
+ private final List<String> _primaryKeyColumns;
+ private final GenericRow _reuse = new GenericRow();
+ private final List<SegmentDataManager> _segmentDataManagers;
+ private final TableDataManager _tableDataManager;
+
+ MemoryOptimizedDimensionTable(Schema tableSchema, List<String> primaryKeyColumns,
+ Map<PrimaryKey, LookupRecordLocation> lookupTable, List<SegmentDataManager> segmentDataManagers,
+ TableDataManager tableDataManager) {
+ _tableSchema = tableSchema;
+ _primaryKeyColumns = primaryKeyColumns;
+ _lookupTable = lookupTable;
+ _segmentDataManagers = segmentDataManagers;
+ _tableDataManager = tableDataManager;
+ }
+
+ @Override
+ public List<String> getPrimaryKeyColumns() {
+ return _primaryKeyColumns;
+ }
+
+ @Override
+ public GenericRow get(PrimaryKey pk) {
+ LookupRecordLocation lookupRecordLocation = _lookupTable.get(pk);
+ if (lookupRecordLocation == null) {
+ return null;
+ }
+ return lookupRecordLocation.getRecord(_reuse);
Review Comment:
I think we need to clear the _reuse before passing it to `.getRecord`.
That's what we do in other places in the code.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] saurabhd336 commented on a diff in pull request #9802: Add memory optimized dimension table
Posted by "saurabhd336 (via GitHub)" <gi...@apache.org>.
saurabhd336 commented on code in PR #9802:
URL: https://github.com/apache/pinot/pull/9802#discussion_r1151475692
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/MemoryOptimizedDimensionTable.java:
##########
@@ -0,0 +1,93 @@
+/**
+ * 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.data.manager.offline;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.segment.local.data.manager.SegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.data.readers.PrimaryKey;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+class MemoryOptimizedDimensionTable implements DimensionTable {
+ private static final Logger LOGGER = LoggerFactory.getLogger(MemoryOptimizedDimensionTable.class);
+
+ private final Map<PrimaryKey, LookupRecordLocation> _lookupTable;
+ private final Schema _tableSchema;
+ private final List<String> _primaryKeyColumns;
+ private final GenericRow _reuse = new GenericRow();
+ private final List<SegmentDataManager> _segmentDataManagers;
+ private final TableDataManager _tableDataManager;
+
+ MemoryOptimizedDimensionTable(Schema tableSchema, List<String> primaryKeyColumns,
+ Map<PrimaryKey, LookupRecordLocation> lookupTable, List<SegmentDataManager> segmentDataManagers,
+ TableDataManager tableDataManager) {
+ _tableSchema = tableSchema;
+ _primaryKeyColumns = primaryKeyColumns;
+ _lookupTable = lookupTable;
+ _segmentDataManagers = segmentDataManagers;
+ _tableDataManager = tableDataManager;
+ }
+
+ @Override
+ public List<String> getPrimaryKeyColumns() {
+ return _primaryKeyColumns;
+ }
+
+ @Override
+ public GenericRow get(PrimaryKey pk) {
+ LookupRecordLocation lookupRecordLocation = _lookupTable.get(pk);
+ if (lookupRecordLocation == null) {
+ return null;
+ }
+ return lookupRecordLocation.getRecord(_reuse);
Review Comment:
Is this thread safe? _reuse being shared across .get calls?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] KKcorps merged pull request #9802: Add memory optimized dimension table
Posted by GitBox <gi...@apache.org>.
KKcorps merged PR #9802:
URL: https://github.com/apache/pinot/pull/9802
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9802: Add memory optimized dimension table
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9802:
URL: https://github.com/apache/pinot/pull/9802#discussion_r1037455060
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -78,16 +81,32 @@ public static DimensionTableDataManager getInstanceByTableName(String tableNameW
AtomicReferenceFieldUpdater.newUpdater(DimensionTableDataManager.class, DimensionTable.class, "_dimensionTable");
private volatile DimensionTable _dimensionTable;
+ private boolean _disablePreload = false;
@Override
protected void doInit() {
super.doInit();
Schema schema = ZKMetadataProvider.getTableSchema(_propertyStore, _tableNameWithType);
Preconditions.checkState(schema != null, "Failed to find schema for dimension table: %s", _tableNameWithType);
+
List<String> primaryKeyColumns = schema.getPrimaryKeyColumns();
Preconditions.checkState(CollectionUtils.isNotEmpty(primaryKeyColumns),
"Primary key columns must be configured for dimension table: %s", _tableNameWithType);
- _dimensionTable = new DimensionTable(schema, primaryKeyColumns);
+
+ TableConfig tableConfig = ZKMetadataProvider.getTableConfig(_propertyStore, _tableNameWithType);
+ if (tableConfig != null) {
+ DimensionTableConfig dimensionTableConfig = tableConfig.getDimensionTableConfig();
+ if (dimensionTableConfig != null) {
+ _disablePreload = dimensionTableConfig.isDisablePreload();
+ }
+ }
+
+ if (_disablePreload) {
+ _dimensionTable = new MemoryOptimizedDimensionTable(schema, primaryKeyColumns, new HashMap<>(),
Review Comment:
(minor) Use `Collections.emptyMap()` and `Collections.emptyList()`
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/MemoryOptimizedDimensionTable.java:
##########
@@ -0,0 +1,93 @@
+/**
+ * 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.data.manager.offline;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.segment.local.data.manager.SegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.data.readers.PrimaryKey;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+class MemoryOptimizedDimensionTable implements DimensionTable {
+ private static final Logger LOGGER = LoggerFactory.getLogger(MemoryOptimizedDimensionTable.class);
+
+ private final Map<PrimaryKey, LookupRecordLocation> _lookupTable;
+ private final Schema _tableSchema;
+ private final List<String> _primaryKeyColumns;
+ private final GenericRow _reuse = new GenericRow();
+ private final List<SegmentDataManager> _segmentDataManagers;
+ private final TableDataManager _tableDataManager;
+
+ MemoryOptimizedDimensionTable(Schema tableSchema, List<String> primaryKeyColumns,
+ Map<PrimaryKey, LookupRecordLocation> lookupTable, List<SegmentDataManager> segmentDataManagers,
+ TableDataManager tableDataManager) {
+ _tableSchema = tableSchema;
+ _primaryKeyColumns = primaryKeyColumns;
+ _lookupTable = lookupTable;
+ _segmentDataManagers = segmentDataManagers;
+ _tableDataManager = tableDataManager;
+ }
+
+ @Override
+ public List<String> getPrimaryKeyColumns() {
+ return _primaryKeyColumns;
+ }
+
+ @Override
+ public GenericRow get(PrimaryKey pk) {
+ LookupRecordLocation lookupRecordLocation = _lookupTable.get(pk);
+ if (lookupRecordLocation == null) {
+ return null;
+ }
+ return lookupRecordLocation.getRecord(_reuse);
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return _lookupTable.isEmpty();
+ }
+
+ @Override
+ public FieldSpec getFieldSpecFor(String columnName) {
+ return _tableSchema.getFieldSpecFor(columnName);
+ }
+
+ @Override
+ public void close()
+ throws IOException {
+ for (LookupRecordLocation lookupRecordLocation: _lookupTable.values()) {
+ try {
+ lookupRecordLocation.getPinotSegmentRecordReader().close();
+ } catch (Exception e) {
+ LOGGER.warn("Cannot close segment record reader", e);
+ }
+ }
+
+ for (SegmentDataManager segmentDataManager: _segmentDataManagers) {
+ _tableDataManager.releaseSegment(segmentDataManager);
Review Comment:
(code format) indentation
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -117,6 +136,19 @@ public void removeSegment(String segmentName) {
}
}
+ @Override
+ protected void doShutdown() {
+ closeDimensionTable(_dimensionTable);
+ }
+
+ private void closeDimensionTable(DimensionTable dimensionTable) {
+ try {
Review Comment:
(code format) indentation
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org