You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2023/04/01 05:06:22 UTC

[kudu-CR](branch-1.17.x) KUDU-1945 Update auto incrementing counter during bootstrap

Hello Kudu Jenkins,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/19678

to review the following change.


Change subject: KUDU-1945 Update auto incrementing counter during bootstrap
......................................................................

KUDU-1945 Update auto incrementing counter during bootstrap

The auto incrementing counter would be reset to zero when the tablet
is being initialized. It is essential to have the correct value of
the counter. There are two cases:

1. WAL segments contain insert operations
In this scenario the WAL segments are replayed and since each insert
operation entry has auto incrementing counter which will be used for
the insert operations present in that entry, as long as we have the
latest insert operation entry in the WAL segments, the auto
incrementing counter is populated correctly during bootstrap.

2. WAL segments do not contain insert operations
In this case, we need to fetch the highest auto incrementing counter
value present in the data which is already flushed and update the
in-memory auto incrementing counter appropriately. This patch
accomplishes this task.

There are tests for the bootstrap scenarios where
1. We have no WAL segments with an INSERT OP containing the auto
incrementing column value. We rely on the auto incrementing counter
present in the data directories in this case.
2. We have no WAL segments with auto incrementing column value
and all the rows of the table are deleted. We reset the auto
incrementing counter in this case.
3. We have non committed replicate ops containing INSERT OPs with
the auto incrementing column values.

Manually tested the time taken to populate the auto incrementing
counter:
Columns - A Non Unique Primary Key column of type INT32
        - 8 INT64 columns
        - 5 STRING columns
Rows - 500k rows with data populated
Time taken in populating the counter during bootstrap:
Min - 235ms
Max - 466ms
Median - 312ms

The total time spent boostrapping the tablet was between 18-25
seconds.

Change-Id: I61b305efd7d5a065a2976327567163956c0c2184
Reviewed-on: http://gerrit.cloudera.org:8080/19445
Reviewed-by: Alexey Serbin <al...@apache.org>
Tested-by: Kudu Jenkins
(cherry picked from commit 8273792156d26c46f788558c896a0729ac2fedd1)
---
M src/kudu/integration-tests/auto_incrementing-itest.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tserver/tablet_server-test-base.h
4 files changed, 336 insertions(+), 42 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/19678/1
-- 
To view, visit http://gerrit.cloudera.org:8080/19678
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I61b305efd7d5a065a2976327567163956c0c2184
Gerrit-Change-Number: 19678
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR](branch-1.17.x) KUDU-1945 Update auto incrementing counter during bootstrap

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19678 )

Change subject: KUDU-1945 Update auto incrementing counter during bootstrap
......................................................................

KUDU-1945 Update auto incrementing counter during bootstrap

The auto incrementing counter would be reset to zero when the tablet
is being initialized. It is essential to have the correct value of
the counter. There are two cases:

1. WAL segments contain insert operations
In this scenario the WAL segments are replayed and since each insert
operation entry has auto incrementing counter which will be used for
the insert operations present in that entry, as long as we have the
latest insert operation entry in the WAL segments, the auto
incrementing counter is populated correctly during bootstrap.

2. WAL segments do not contain insert operations
In this case, we need to fetch the highest auto incrementing counter
value present in the data which is already flushed and update the
in-memory auto incrementing counter appropriately. This patch
accomplishes this task.

There are tests for the bootstrap scenarios where
1. We have no WAL segments with an INSERT OP containing the auto
incrementing column value. We rely on the auto incrementing counter
present in the data directories in this case.
2. We have no WAL segments with auto incrementing column value
and all the rows of the table are deleted. We reset the auto
incrementing counter in this case.
3. We have non committed replicate ops containing INSERT OPs with
the auto incrementing column values.

Manually tested the time taken to populate the auto incrementing
counter:
Columns - A Non Unique Primary Key column of type INT32
        - 8 INT64 columns
        - 5 STRING columns
Rows - 500k rows with data populated
Time taken in populating the counter during bootstrap:
Min - 235ms
Max - 466ms
Median - 312ms

The total time spent boostrapping the tablet was between 18-25
seconds.

Change-Id: I61b305efd7d5a065a2976327567163956c0c2184
Reviewed-on: http://gerrit.cloudera.org:8080/19445
Reviewed-by: Alexey Serbin <al...@apache.org>
Tested-by: Kudu Jenkins
(cherry picked from commit 8273792156d26c46f788558c896a0729ac2fedd1)
Reviewed-on: http://gerrit.cloudera.org:8080/19678
Reviewed-by: Yingchun Lai <la...@apache.org>
Tested-by: Yingchun Lai <la...@apache.org>
---
M src/kudu/integration-tests/auto_incrementing-itest.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tserver/tablet_server-test-base.h
4 files changed, 336 insertions(+), 42 deletions(-)

Approvals:
  Yingchun Lai: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/19678
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I61b305efd7d5a065a2976327567163956c0c2184
Gerrit-Change-Number: 19678
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>

[kudu-CR](branch-1.17.x) KUDU-1945 Update auto incrementing counter during bootstrap

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19678 )

Change subject: KUDU-1945 Update auto incrementing counter during bootstrap
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19678/1/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/19678/1/src/kudu/tablet/tablet.cc@442
PS1, Line 442: :
> nit: add an extra space.
Done.

Usually, cherry-picks should go as-is, but here I updated this as directed.



-- 
To view, visit http://gerrit.cloudera.org:8080/19678
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I61b305efd7d5a065a2976327567163956c0c2184
Gerrit-Change-Number: 19678
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Sun, 02 Apr 2023 20:28:15 +0000
Gerrit-HasComments: Yes

[kudu-CR](branch-1.17.x) KUDU-1945 Update auto incrementing counter during bootstrap

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19678 )

Change subject: KUDU-1945 Update auto incrementing counter during bootstrap
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19678/1/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/19678/1/src/kudu/tablet/tablet.cc@442
PS1, Line 442: "
nit: add an extra space.



-- 
To view, visit http://gerrit.cloudera.org:8080/19678
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I61b305efd7d5a065a2976327567163956c0c2184
Gerrit-Change-Number: 19678
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Sat, 01 Apr 2023 07:02:22 +0000
Gerrit-HasComments: Yes

[kudu-CR](branch-1.17.x) KUDU-1945 Update auto incrementing counter during bootstrap

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has removed a vote on this change.

Change subject: KUDU-1945 Update auto incrementing counter during bootstrap
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/19678
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I61b305efd7d5a065a2976327567163956c0c2184
Gerrit-Change-Number: 19678
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>

[kudu-CR](branch-1.17.x) KUDU-1945 Update auto incrementing counter during bootstrap

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19678 )

Change subject: KUDU-1945 Update auto incrementing counter during bootstrap
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/19678
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I61b305efd7d5a065a2976327567163956c0c2184
Gerrit-Change-Number: 19678
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Mon, 03 Apr 2023 02:10:44 +0000
Gerrit-HasComments: No

[kudu-CR](branch-1.17.x) KUDU-1945 Update auto incrementing counter during bootstrap

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19678 )

Change subject: KUDU-1945 Update auto incrementing counter during bootstrap
......................................................................


Patch Set 2: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/19678
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I61b305efd7d5a065a2976327567163956c0c2184
Gerrit-Change-Number: 19678
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Mon, 03 Apr 2023 02:11:07 +0000
Gerrit-HasComments: No

[kudu-CR](branch-1.17.x) KUDU-1945 Update auto incrementing counter during bootstrap

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins, Abhishek Chennaka, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19678

to look at the new patch set (#2).

Change subject: KUDU-1945 Update auto incrementing counter during bootstrap
......................................................................

KUDU-1945 Update auto incrementing counter during bootstrap

The auto incrementing counter would be reset to zero when the tablet
is being initialized. It is essential to have the correct value of
the counter. There are two cases:

1. WAL segments contain insert operations
In this scenario the WAL segments are replayed and since each insert
operation entry has auto incrementing counter which will be used for
the insert operations present in that entry, as long as we have the
latest insert operation entry in the WAL segments, the auto
incrementing counter is populated correctly during bootstrap.

2. WAL segments do not contain insert operations
In this case, we need to fetch the highest auto incrementing counter
value present in the data which is already flushed and update the
in-memory auto incrementing counter appropriately. This patch
accomplishes this task.

There are tests for the bootstrap scenarios where
1. We have no WAL segments with an INSERT OP containing the auto
incrementing column value. We rely on the auto incrementing counter
present in the data directories in this case.
2. We have no WAL segments with auto incrementing column value
and all the rows of the table are deleted. We reset the auto
incrementing counter in this case.
3. We have non committed replicate ops containing INSERT OPs with
the auto incrementing column values.

Manually tested the time taken to populate the auto incrementing
counter:
Columns - A Non Unique Primary Key column of type INT32
        - 8 INT64 columns
        - 5 STRING columns
Rows - 500k rows with data populated
Time taken in populating the counter during bootstrap:
Min - 235ms
Max - 466ms
Median - 312ms

The total time spent boostrapping the tablet was between 18-25
seconds.

Change-Id: I61b305efd7d5a065a2976327567163956c0c2184
Reviewed-on: http://gerrit.cloudera.org:8080/19445
Reviewed-by: Alexey Serbin <al...@apache.org>
Tested-by: Kudu Jenkins
(cherry picked from commit 8273792156d26c46f788558c896a0729ac2fedd1)
---
M src/kudu/integration-tests/auto_incrementing-itest.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tserver/tablet_server-test-base.h
4 files changed, 336 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/19678/2
-- 
To view, visit http://gerrit.cloudera.org:8080/19678
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I61b305efd7d5a065a2976327567163956c0c2184
Gerrit-Change-Number: 19678
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>