Storages: introduce inverted index file format & writer & reader #9844

Lloyd-Pottiger · 2025-02-06T08:31:40Z

What problem does this PR solve?

Issue Number: ref #9843

Problem Summary:

What is changed and how it works?

First part of inverted index, introduce inverted index file format & builder & viewer

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

None

ti-chi-bot · 2025-02-06T08:31:47Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from lloyd-pottiger, ensuring that each of them provides their approval before proceeding. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

dbms/src/Storages/DeltaMerge/dtpb/index_file.proto

dbms/src/Storages/DeltaMerge/Index/LocalIndexInfo.h

dbms/src/Storages/DeltaMerge/Index/LocalIndexInfo.cpp

dbms/src/Storages/DeltaMerge/Index/LocalIndexBuilder.h

dbms/src/Storages/DeltaMerge/File/DMFile.h

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileTinyLocalIndexWriter.cpp

dbms/src/Storages/DeltaMerge/Index/LocalIndexInfo.cpp

JinheLin · 2025-03-03T02:59:22Z

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileTinyLocalIndexWriter.cpp

-                RUNTIME_CHECK_MSG(false, "Unsupported index kind: {}", magic_enum::enum_name(index.info.kind));
-                break;
-            }
+            if (auto builder = LocalIndexBuilder::create(index.info); builder)


What if builder is nullptr? Should we at least print some logs for debugging.

dbms/src/Storages/DeltaMerge/Index/LocalIndexWriter.h

breezewish

Index framework part looks fine

dbms/src/Storages/DeltaMerge/Index/LocalIndexWriter.h

dbms/src/Storages/DeltaMerge/tests/gtest_dm_vector_index.cpp

CalvinNeo · 2025-03-07T05:23:10Z

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileTinyLocalIndexWriter.cpp

+            auto data_size = write_buf.count();
+            auto buf = write_buf.tryGetReadBuffer();
+            // ColumnFileDataProviderRNLocalPageCache currently does not support read data with fields
+            options.wbs.log.putPage(index_page_id, 0, buf, data_size, {data_size});


What magnitude is the size of the page for the inverted index going to be? Is it BlockSize or times of BlockSize?

AfterCompressed(MetaSize + BlockCount * BlockSize)

Why do we need to set data_sizes if we always read the whole page from disk?

ColumnFileDataProviderRNLocalPageCache currently does not support read data withiout fields

CalvinNeo · 2025-03-07T05:35:20Z

dbms/src/Storages/DeltaMerge/Index/InvertedIndex/CommonUtil.cpp

+    {
+        auto & entry = block.entries[i];
+        read_buf.read(reinterpret_cast<char *>(entry.row_ids.data()), entry.row_ids.size() * sizeof(RowID));
+    }


I am not sure if it is good handle some EOF failures here? Because I can only expect a data corruption happens here.

I will use readStrict instead.

dbms/src/Storages/DeltaMerge/Index/InvertedIndex/CommonUtil.cpp

CalvinNeo · 2025-03-07T05:46:20Z

dbms/src/Storages/DeltaMerge/Index/InvertedIndex/CommonUtil.cpp

+        block.entries[i].value = value;
+        block.entries[i].row_ids.resize(row_ids_size);
+    }
+    for (UInt32 i = 0; i < size; ++i)


I have a naive question... Is there going to be some cases where we only need the row values here? For example, if we want a count in given range, then seems we don't need the actual row_ids then?
If so, we can save some memory here, and make the local_index_cache bigger to reduce its possibilities of being evicted.

We can not support agg now.

We can't because we don't have enough time, or we can't because the arch doesn't support?

We don't have enough time. We have stored the size of row ids.

CalvinNeo · 2025-03-07T05:50:51Z

dbms/src/Common/TiFlashMetrics.h

+      "Inverted index operation duration",                                                                                          \
+      Histogram,                                                                                                                    \
+      F(type_build, {{"type", "build"}}, ExpBuckets{0.001, 2, 20}),                                                                 \
+      F(type_download, {{"type", "download"}}, ExpBuckets{0.001, 2, 20}),                                                           \


Where will change this metric?

build: ~InvertedIndexWriterInternal
download: will used in DMFileInvertedIndexReader which is not included in this PR.

breezewish

The rest looks good

dbms/src/Storages/DeltaMerge/Index/InvertedIndex/CommonUtil.cpp

breezewish · 2025-03-07T03:44:30Z

dbms/src/Storages/DeltaMerge/Index/InvertedIndex/CommonUtil.cpp

+}
+
+template <typename T>
+void Block<T>::search(BitmapFilterPtr & bitmap_filter, ReadBuffer & read_buf, T key)


I'm worried about the performance as it involves a lot of syscall (even though the underlying page is possibly cached).

The buffer size is 1MB by default, so maybe it is acceptable.

I think it is a pure looking-forward scene, so the buffer should be adequate.

breezewish · 2025-03-07T03:52:28Z

dbms/src/Storages/DeltaMerge/Index/InvertedIndex/CommonUtil.cpp

+{
+    UInt32 size;
+    readIntBinary(size, read_buf);
+    UInt32 seek_offset = size * (sizeof(T) + sizeof(UInt32));


Interesting, why not simply use an absolute seek? Could be possibly make it simpler (whence=SEEK_SET)

ReadBuffer does not support seek

breezewish · 2025-03-07T06:25:44Z

dbms/src/Storages/DeltaMerge/Index/InvertedIndex/Reader.cpp

+    T real_key = key;
+    auto it = index.find(real_key);
+    if (it != index.end())
+        bitmap_filter->set(it->second, nullptr);


Existing values in bitmap_filter is not cleared. Is it ok?

breezewish · 2025-03-07T06:26:14Z

dbms/src/Storages/DeltaMerge/Index/InvertedIndex/Reader.cpp

+}
+
+template <typename T>
+void InvertedIndexMemoryReader<T>::searchRange(BitmapFilterPtr & bitmap_filter, const Key & begin, const Key & end)


Is this used for SQLS like WHERE x >= .. and x <= ..?

Yes.

x > 0 ==> [1, MAX]

x < 10 ==> [MIN, 9]

x > 0 & x < 10 ==> [1, 9]

breezewish · 2025-03-07T06:26:42Z

dbms/src/Storages/DeltaMerge/Index/InvertedIndex/Reader.cpp

+    case TypeIndex::MyDate:
+    case TypeIndex::MyDateTime:
+    case TypeIndex::MyTimeStamp:
+        return std::make_shared<InvertedIndexMemoryReader<UInt64>>(buf, index_size);


Why do they decay to UInt64? Are there any references

tiflash/dbms/src/TiDB/Schema/TiDBTypes.h

Lines 26 to 55 in 3cda2f6

#define COLUMN_TYPES(M) \

M(Decimal, 0, Decimal, Decimal32) \

M(Tiny, 1, VarInt, Int8) \

M(Short, 2, VarInt, Int16) \

M(Long, 3, VarInt, Int32) \

M(Float, 4, Float, Float32) \

M(Double, 5, Float, Float64) \

M(Null, 6, Nil, Nothing) \

M(Timestamp, 7, UInt, MyDateTime) \

M(LongLong, 8, Int, Int64) \

M(Int24, 9, VarInt, Int32) \

M(Date, 10, UInt, MyDate) \

M(Time, 11, Duration, Int64) \

M(Datetime, 12, UInt, MyDateTime) \

M(Year, 13, Int, Int16) \

M(NewDate, 14, Int, MyDate) \

M(Varchar, 15, CompactBytes, String) \

M(Bit, 16, VarInt, UInt64) \

M(JSON, 0xf5, Json, String) \

M(NewDecimal, 0xf6, Decimal, Decimal32) \

M(Enum, 0xf7, VarUInt, Enum16) \

M(Set, 0xf8, VarUInt, UInt64) \

M(TinyBlob, 0xf9, CompactBytes, String) \

M(MediumBlob, 0xfa, CompactBytes, String) \

M(LongBlob, 0xfb, CompactBytes, String) \

M(Blob, 0xfc, CompactBytes, String) \

M(VarString, 0xfd, CompactBytes, String) \

M(String, 0xfe, CompactBytes, String) \

M(Geometry, 0xff, CompactBytes, String) \

M(TiDBVectorFloat32, 0xe1, VectorFloat32, Array)

tiflash/dbms/src/DataTypes/DataTypeMyTimeBase.h

Line 23 in 3cda2f6

class DataTypeMyTimeBase : public DataTypeNumberBase<UInt64>

dbms/src/Storages/DeltaMerge/File/DMFileLocalIndexWriter.cpp

JaySon-Huang · 2025-03-10T02:54:20Z

dbms/src/Storages/DeltaMerge/Index/LocalIndexInfo.cpp

-                    // Only one of the below will be set
-                    .def_vector_index = idx.vector_index,
-                });
+                new_index_infos->emplace_back(LocalIndexInfo(idx.id, column_id, idx.vector_index));


Will the logic for adding inverted index be added in a later PR?

JaySon-Huang · 2025-03-10T03:20:54Z

dbms/src/Storages/DeltaMerge/ColumnFile/ColumnFileTinyLocalIndexWriter.cpp

+            auto data_size = write_buf.count();
+            auto buf = write_buf.tryGetReadBuffer();
+            // ColumnFileDataProviderRNLocalPageCache currently does not support read data with fields
+            options.wbs.log.putPage(index_page_id, 0, buf, data_size, {data_size});


Why do we need to set data_sizes if we always read the whole page from disk?

dbms/src/Storages/DeltaMerge/Index/InvertedIndex/CommonUtil.h

Signed-off-by: Lloyd-Pottiger <[email protected]>

ti-chi-bot bot added the release-note-none Denotes a PR that doesn't merit a release note. label Feb 6, 2025

ti-chi-bot bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 6, 2025

Lloyd-Pottiger force-pushed the lib-inverted-index branch from 9dcf222 to df58e9d Compare February 6, 2025 09:28

Lloyd-Pottiger force-pushed the lib-inverted-index branch from df58e9d to 17a1b6c Compare February 28, 2025 07:22

Lloyd-Pottiger requested review from JaySon-Huang, JinheLin and CalvinNeo February 28, 2025 07:24

breezewish reviewed Mar 1, 2025

View reviewed changes

JinheLin reviewed Mar 3, 2025

View reviewed changes

Lloyd-Pottiger force-pushed the lib-inverted-index branch from 17a1b6c to 07ebbf2 Compare March 3, 2025 09:38

Lloyd-Pottiger changed the title ~~Storages: introduce inverted index file format & builder & viewer~~ Storages: introduce inverted index file format & writer & reader Mar 3, 2025

Lloyd-Pottiger requested review from breezewish and JinheLin March 3, 2025 09:40

breezewish reviewed Mar 4, 2025

View reviewed changes

dbms/src/Storages/DeltaMerge/Index/LocalIndexWriter.h Outdated Show resolved Hide resolved

breezewish reviewed Mar 4, 2025

View reviewed changes

dbms/src/Storages/DeltaMerge/Index/LocalIndexWriter.h Show resolved Hide resolved

Lloyd-Pottiger force-pushed the lib-inverted-index branch from 619ff99 to 4d11099 Compare March 4, 2025 07:11

breezewish reviewed Mar 5, 2025

View reviewed changes

Lloyd-Pottiger force-pushed the lib-inverted-index branch from 804493d to d35a94a Compare March 6, 2025 05:53

CalvinNeo reviewed Mar 7, 2025

View reviewed changes

dbms/src/Storages/DeltaMerge/Index/InvertedIndex/CommonUtil.cpp Show resolved Hide resolved

CalvinNeo reviewed Mar 7, 2025

View reviewed changes

breezewish reviewed Mar 7, 2025

View reviewed changes

JaySon-Huang reviewed Mar 10, 2025

View reviewed changes

dbms/src/Storages/DeltaMerge/Index/InvertedIndex/CommonUtil.h Outdated Show resolved Hide resolved

JaySon-Huang reviewed Mar 10, 2025

View reviewed changes

dbms/src/Storages/DeltaMerge/Index/InvertedIndex/CommonUtil.h Outdated Show resolved Hide resolved

Lloyd-Pottiger added 2 commits March 10, 2025 14:20

Storages: introduce inverted index file format & builder & viewer

5f84db6

Signed-off-by: Lloyd-Pottiger <[email protected]>

refine

310b0b7

Signed-off-by: Lloyd-Pottiger <[email protected]>

Lloyd-Pottiger added 4 commits March 10, 2025 14:20

refine

12a4f1b

Signed-off-by: Lloyd-Pottiger <[email protected]>

address comments

4eaf2b4

Signed-off-by: Lloyd-Pottiger <[email protected]>

address comments & add version

eb7192a

Signed-off-by: Lloyd-Pottiger <[email protected]>

address comments

164d2a3

Signed-off-by: Lloyd-Pottiger <[email protected]>

Lloyd-Pottiger force-pushed the lib-inverted-index branch from d35a94a to 164d2a3 Compare March 10, 2025 06:31

address comments

0b433ec

Signed-off-by: Lloyd-Pottiger <[email protected]>

Lloyd-Pottiger requested review from breezewish, JaySon-Huang and CalvinNeo March 10, 2025 09:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Storages: introduce inverted index file format & writer & reader #9844

Storages: introduce inverted index file format & writer & reader #9844

Lloyd-Pottiger commented Feb 6, 2025 •

edited

Loading

ti-chi-bot bot commented Feb 6, 2025

JinheLin Mar 3, 2025

breezewish left a comment

CalvinNeo Mar 7, 2025 •

edited

Loading

Lloyd-Pottiger Mar 7, 2025

JaySon-Huang Mar 10, 2025

Lloyd-Pottiger Mar 10, 2025

CalvinNeo Mar 7, 2025

Lloyd-Pottiger Mar 7, 2025

CalvinNeo Mar 7, 2025

Lloyd-Pottiger Mar 7, 2025 •

edited

Loading

CalvinNeo Mar 7, 2025

Lloyd-Pottiger Mar 10, 2025

CalvinNeo Mar 7, 2025

Lloyd-Pottiger Mar 7, 2025

breezewish left a comment

breezewish Mar 7, 2025

Lloyd-Pottiger Mar 7, 2025

CalvinNeo Mar 8, 2025

breezewish Mar 7, 2025

Lloyd-Pottiger Mar 7, 2025

breezewish Mar 7, 2025

Lloyd-Pottiger Mar 7, 2025

breezewish Mar 7, 2025

Lloyd-Pottiger Mar 7, 2025

breezewish Mar 7, 2025

Lloyd-Pottiger Mar 7, 2025

Lloyd-Pottiger Mar 7, 2025

JaySon-Huang Mar 10, 2025

Lloyd-Pottiger Mar 10, 2025

JaySon-Huang Mar 10, 2025

	#define COLUMN_TYPES(M) \
	M(Decimal, 0, Decimal, Decimal32) \
	M(Tiny, 1, VarInt, Int8) \
	M(Short, 2, VarInt, Int16) \
	M(Long, 3, VarInt, Int32) \
	M(Float, 4, Float, Float32) \
	M(Double, 5, Float, Float64) \
	M(Null, 6, Nil, Nothing) \
	M(Timestamp, 7, UInt, MyDateTime) \
	M(LongLong, 8, Int, Int64) \
	M(Int24, 9, VarInt, Int32) \
	M(Date, 10, UInt, MyDate) \
	M(Time, 11, Duration, Int64) \
	M(Datetime, 12, UInt, MyDateTime) \
	M(Year, 13, Int, Int16) \
	M(NewDate, 14, Int, MyDate) \
	M(Varchar, 15, CompactBytes, String) \
	M(Bit, 16, VarInt, UInt64) \
	M(JSON, 0xf5, Json, String) \
	M(NewDecimal, 0xf6, Decimal, Decimal32) \
	M(Enum, 0xf7, VarUInt, Enum16) \
	M(Set, 0xf8, VarUInt, UInt64) \
	M(TinyBlob, 0xf9, CompactBytes, String) \
	M(MediumBlob, 0xfa, CompactBytes, String) \
	M(LongBlob, 0xfb, CompactBytes, String) \
	M(Blob, 0xfc, CompactBytes, String) \
	M(VarString, 0xfd, CompactBytes, String) \
	M(String, 0xfe, CompactBytes, String) \
	M(Geometry, 0xff, CompactBytes, String) \
	M(TiDBVectorFloat32, 0xe1, VectorFloat32, Array)

Storages: introduce inverted index file format & writer & reader #9844

Are you sure you want to change the base?

Storages: introduce inverted index file format & writer & reader #9844

Conversation

Lloyd-Pottiger commented Feb 6, 2025 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot bot commented Feb 6, 2025

Choose a reason for hiding this comment

breezewish left a comment

Choose a reason for hiding this comment

CalvinNeo Mar 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lloyd-Pottiger Mar 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

breezewish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lloyd-Pottiger commented Feb 6, 2025 •

edited

Loading

CalvinNeo Mar 7, 2025 •

edited

Loading

Lloyd-Pottiger Mar 7, 2025 •

edited

Loading