IGNITE-17855 Implement inline size calculation for B+tree #1179

tkalkirill · 2022-10-07T17:59:03Z

https://issues.apache.org/jira/browse/IGNITE-17855

AMashenkov · 2022-10-07T22:54:43Z

modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/InlineUtils.java

+            case STRING: {
+                int length = ((VarlenNativeType) nativeType).length();
+
+                return length == Integer.MAX_VALUE ? UNDEFINED_VARLEN_INLINE_SIZE : length * 2;


Suggested change

return length == Integer.MAX_VALUE ? UNDEFINED_VARLEN_INLINE_SIZE : length * 2;

return length == Integer.MAX_VALUE ? UNDEFINED_VARLEN_INLINE_SIZE : length;

Do you expect most of users will use 2 bytes chars?
Why should an index over columns of ASCII chars be twice large by default?
And what to do with 4-byte chars?

I proceed from the fact that in Java characters is char (2 bytes).
We can assume, as in Java, that in most cases the characters will be ascii (1 byte).
We're just doing an approximate inline count, I don't know how often we'll see 4 byte characters.

Later we can add an encoding (utf-8, 16, 32) to the configuration or ddl.

wdyt?

ok, also I'd limit default value for var-length types (e.g. with 64-256 bytes).

Usually, the upper limit for var-length columns is usually calculated with some reserve.
So, the actual values size is less the limit (often noticeably).
Also, large inline-size means less data will fit to the index page and may have negative impact on performance.

In case of relatively large values with similar prefixes user always can increase inline-size manually,
but imho, in most of cases, this limit will reduce index size with no performance penalty.

...ge-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtils.java

sashapolo · 2022-10-10T15:01:56Z

...ge-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtils.java

+/**
+ * Helper class for index inlining.
+ */
+public class InlineUtils {


Most methods in this class a package-private, while this class is public.

This class will be used in VolatilePageMemoryTableStorage and PersistentPageMemoryTableStorage.

I guess they're package-private for tests, right?

...ge-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtils.java

...es/page-memory/src/main/java/org/apache/ignite/internal/pagememory/tree/io/BplusInnerIo.java

...ge-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtils.java

ibessonov · 2022-10-11T07:44:36Z

...ge-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtils.java

+/**
+ * Helper class for index inlining.
+ */
+public class InlineUtils {


I guess they're package-private for tests, right?

...ge-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtils.java

ibessonov · 2022-10-11T08:01:37Z

...emory/src/test/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtilsTest.java

+        assertEquals(9, inlineSize(nativeType = NativeTypes.datetime()));
+        nativeTypeSpecs.remove(nativeType.spec());
+
+        assertEquals(12, inlineSize(nativeType = NativeTypes.timestamp()));


12 bytes per timestamp? Jesus! Why is it so big? Just asking

it's seconds (long) + nanos (int)

...emory/src/test/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtilsTest.java

ibessonov · 2022-10-11T08:06:19Z

...emory/src/test/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtilsTest.java

+    void testBinaryTupleInlineSize() {
+        IndexDescriptor indexDescriptor = testIndexDescriptor();
+
+        assertEquals(BinaryTupleCommon.HEADER_SIZE, binaryTupleInlineSize(indexDescriptor));


This is funny, you know. I'd expect an exception, who would want to make a tuple with 0 columns.
On the other hand, can this be useful somewhere? Just asking

I don't know yet, I can add an assertion

ibessonov

You have my approval, let's wait for other guys now

...ge-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtils.java

modules/storage-api/src/main/java/org/apache/ignite/internal/storage/index/IndexDescriptor.java

...ge-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtils.java

tkalkirill added 3 commits October 7, 2022 20:58

IGNITE-17855 wip

d5a644a

IGNITE-17855 wip

1509c97

IGNITE-17855 wip

b0dd2b7

AMashenkov reviewed Oct 7, 2022

View reviewed changes

tkalkirill added 4 commits October 10, 2022 11:40

IGNITE-17855 wip

3b69d8a

Merge branch 'ai-main' into ignite-17855

dc5f753

IGNITE-17855 wip

b84bb3a

Merge branch 'ai-main' into ignite-17855

06d9552

ibessonov reviewed Oct 10, 2022

View reviewed changes

sashapolo reviewed Oct 10, 2022

View reviewed changes

tkalkirill added 3 commits October 10, 2022 23:37

IGNITE-17855 wip

596fd1f

IGNITE-17855 wip

868e8b0

Merge branch 'ai-main' into ignite-17855

e1b366f

ibessonov reviewed Oct 11, 2022

View reviewed changes

IGNITE-17855 after review

820cd57

ibessonov approved these changes Oct 11, 2022

View reviewed changes

AMashenkov reviewed Oct 11, 2022

View reviewed changes

...ge-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtils.java Show resolved Hide resolved

AMashenkov reviewed Oct 11, 2022

View reviewed changes

...ge-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtils.java Show resolved Hide resolved

AMashenkov reviewed Oct 11, 2022

View reviewed changes

...ge-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/InlineUtils.java Outdated Show resolved Hide resolved

rpuch reviewed Oct 11, 2022

View reviewed changes

rpuch approved these changes Oct 11, 2022

View reviewed changes

tkalkirill added 3 commits October 11, 2022 15:20

IGNITE-17855 after review

5016ec9

Merge branch 'ai-main' into ignite-17855

3584e3c

IGNITE-17855 after review

d35256f

AMashenkov approved these changes Oct 11, 2022

View reviewed changes

tkalkirill merged commit 215ea8c into apache:main Oct 11, 2022

tkalkirill deleted the ignite-17855 branch October 11, 2022 13:57

lowka pushed a commit to gridgain/apache-ignite-3 that referenced this pull request Mar 18, 2023

IGNITE-17855 Implement inline size calculation for B+tree (apache#1179)

d7ba16d

lowka pushed a commit to gridgain/apache-ignite-3 that referenced this pull request Apr 19, 2023

IGNITE-17855 Implement inline size calculation for B+tree (apache#1179)

17b258f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IGNITE-17855 Implement inline size calculation for B+tree #1179

IGNITE-17855 Implement inline size calculation for B+tree #1179

tkalkirill commented Oct 7, 2022

AMashenkov Oct 7, 2022 •

edited

Loading

tkalkirill Oct 10, 2022

AMashenkov Oct 10, 2022

sashapolo Oct 10, 2022

tkalkirill Oct 10, 2022

ibessonov Oct 11, 2022

tkalkirill Oct 11, 2022

ibessonov Oct 11, 2022

ibessonov Oct 11, 2022

tkalkirill Oct 11, 2022 •

edited

Loading

ibessonov Oct 11, 2022

tkalkirill Oct 11, 2022

ibessonov left a comment

	return length == Integer.MAX_VALUE ? UNDEFINED_VARLEN_INLINE_SIZE : length * 2;
	return length == Integer.MAX_VALUE ? UNDEFINED_VARLEN_INLINE_SIZE : length;

IGNITE-17855 Implement inline size calculation for B+tree #1179

IGNITE-17855 Implement inline size calculation for B+tree #1179

Conversation

tkalkirill commented Oct 7, 2022

AMashenkov Oct 7, 2022 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkalkirill Oct 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibessonov left a comment

Choose a reason for hiding this comment

AMashenkov Oct 7, 2022 •

edited

Loading

tkalkirill Oct 11, 2022 •

edited

Loading