Re: [Maria-developers] 74b2eba1ca6: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING

newer
[Maria-developers] EITS member...

older
Re: [Maria-developers]...

Sergei Golubchik

26 Mar 2019 26 Mar '19

6:19 p.m.

Hi, Aleksey! On Mar 26, Aleksey Midenkov wrote:

...

revision-id: 74b2eba1ca6 (mariadb-10.3.12-115-g74b2eba1ca6) parent(s): 2c0901d808b author: Aleksey Midenkov committer: Sergei Golubchik timestamp: 2019-03-26 17:43:38 +0100 message:

MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING

* HA_EXTRA_REMEMBER_POS, HA_EXTRA_RESTORE_POS for HEAP storage engine * Versioning tests support

diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index abe4254db3e..1d51947cc86 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -1643,7 +1643,12 @@ int vers_insert_history_row(TABLE *table) if (row_start->cmp(row_start->ptr, row_end->ptr) >= 0) return 0;

- return table->file->ha_write_row(table->record[0]); + int res; + if ((res= table->file->extra(HA_EXTRA_REMEMBER_POS))) + return res; + if ((res= table->file->ha_write_row(table->record[0]))) + return res; + return table->file->extra(HA_EXTRA_RESTORE_POS); }

Frankly, I don't like it. I mean, this particular fix is fine, but here's the problem: We now have (at least) three features where the server might need to do something (insert/update/search) during the index/rnd scan. And three different solutions: you use HA_EXTRA_REMEMBER_POS in system versioning, Nikita simply sets delete_while_scanning=false in application time period tables, and Sachin uses handler::clone() to create a second handler that can be used to check for hash collisions without disrupting the scan on the primary handler. I think it's getting somewhat out of hands. Can we please reduce the number of approaches to fix the same issue? Regards, Sergei Chief Architect MariaDB and security@mariadb.org

Show replies by date

Aleksey Midenkov

27 Mar 27 Mar

11:37 a.m.

New subject: [Maria-developers] 74b2eba1ca6: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING

Hi, Sergei! On Wed, Mar 27, 2019 at 1:04 AM Sergei Golubchik wrote:

...

Hi, Aleksey!

On Mar 26, Aleksey Midenkov wrote:

...
revision-id: 74b2eba1ca6 (mariadb-10.3.12-115-g74b2eba1ca6) parent(s): 2c0901d808b author: Aleksey Midenkov committer: Sergei Golubchik timestamp: 2019-03-26 17:43:38 +0100 message:

MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING

* HA_EXTRA_REMEMBER_POS, HA_EXTRA_RESTORE_POS for HEAP storage engine * Versioning tests support

diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index abe4254db3e..1d51947cc86 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -1643,7 +1643,12 @@ int vers_insert_history_row(TABLE *table) if (row_start->cmp(row_start->ptr, row_end->ptr) >= 0) return 0;

- return table->file->ha_write_row(table->record[0]); + int res; + if ((res= table->file->extra(HA_EXTRA_REMEMBER_POS))) + return res; + if ((res= table->file->ha_write_row(table->record[0]))) + return res; + return table->file->extra(HA_EXTRA_RESTORE_POS); }

Frankly, I don't like it. I mean, this particular fix is fine, but here's the problem:

We now have (at least) three features where the server might need to do something (insert/update/search) during the index/rnd scan.

And three different solutions: you use HA_EXTRA_REMEMBER_POS in system versioning, Nikita simply sets delete_while_scanning=false in application time period tables, and Sachin uses handler::clone() to create a second handler that can be used to check for hash collisions without disrupting the scan on the primary handler.

I think it's getting somewhat out of hands. Can we please reduce the number of approaches to fix the same issue?

What solution do you propose to use?

...

Regards, Sergei Chief Architect MariaDB and security@mariadb.org

-- All the best, Aleksey Midenkov @midenok

Nikita Malyavin

12:14 p.m.

New subject: [Maria-developers] 74b2eba1ca6: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING

I personally like this one more then mine. BTW I also clone the handler to do something similar in WITHOUT OVERLAPS. Maybe I should also consider to rewrite it to HA_EXTRA_REMEMBER_POS On Wed, 27 Mar 2019 at 20:37, Aleksey Midenkov wrote:

...

Hi, Sergei!

On Wed, Mar 27, 2019 at 1:04 AM Sergei Golubchik wrote:

...
Hi, Aleksey!

On Mar 26, Aleksey Midenkov wrote:

...
revision-id: 74b2eba1ca6 (mariadb-10.3.12-115-g74b2eba1ca6) parent(s): 2c0901d808b author: Aleksey Midenkov committer: Sergei Golubchik timestamp: 2019-03-26 17:43:38 +0100 message:

MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING

* HA_EXTRA_REMEMBER_POS, HA_EXTRA_RESTORE_POS for HEAP storage engine * Versioning tests support

diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index abe4254db3e..1d51947cc86 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -1643,7 +1643,12 @@ int vers_insert_history_row(TABLE *table) if (row_start->cmp(row_start->ptr, row_end->ptr) >= 0) return 0;

- return table->file->ha_write_row(table->record[0]); + int res; + if ((res= table->file->extra(HA_EXTRA_REMEMBER_POS))) + return res; + if ((res= table->file->ha_write_row(table->record[0]))) + return res; + return table->file->extra(HA_EXTRA_RESTORE_POS); }

Frankly, I don't like it. I mean, this particular fix is fine, but here's the problem:

We now have (at least) three features where the server might need to do something (insert/update/search) during the index/rnd scan.

And three different solutions: you use HA_EXTRA_REMEMBER_POS in system versioning, Nikita simply sets delete_while_scanning=false in application time period tables, and Sachin uses handler::clone() to create a second handler that can be used to check for hash collisions without disrupting the scan on the primary handler.

I think it's getting somewhat out of hands. Can we please reduce the number of approaches to fix the same issue?

What solution do you propose to use?

...
Regards, Sergei Chief Architect MariaDB and security@mariadb.org

-- All the best,

Aleksey Midenkov @midenok

-- Yours truly, Nikita Malyavin

Sergei Golubchik

29 Mar 29 Mar

3:11 p.m.

New subject: [Maria-developers] 74b2eba1ca6: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING

Hi, Aleksey! It seems that everyone prefers your approach :) I also think this it's easier to use and it's a more lightweight than the clone. A clone is justified when one wants to do two scans in parallel (like, read few rows from one handler, read few rows from the clone, than from the first handler, and so on). Which is not the case here, so the clone looks like an overkill. The only problem here - not all engines might implement HA_EXTRA_REMEMBER_POS. But it looks like it's generally easy to implement. On Mar 27, Aleksey Midenkov wrote:

...

...
...
- return table->file->ha_write_row(table->record[0]); + int res; + if ((res= table->file->extra(HA_EXTRA_REMEMBER_POS))) + return res; + if ((res= table->file->ha_write_row(table->record[0]))) + return res; + return table->file->extra(HA_EXTRA_RESTORE_POS); }

Frankly, I don't like it. I mean, this particular fix is fine, but here's the problem:

We now have (at least) three features where the server might need to do something (insert/update/search) during the index/rnd scan.

And three different solutions: you use HA_EXTRA_REMEMBER_POS in system versioning, Nikita simply sets delete_while_scanning=false in application time period tables, and Sachin uses handler::clone() to create a second handler that can be used to check for hash collisions without disrupting the scan on the primary handler.

I think it's getting somewhat out of hands. Can we please reduce the number of approaches to fix the same issue?

What solution do you propose to use?

Regards, Sergei Chief Architect MariaDB and security@mariadb.org

sachin setiya

12:08 a.m.

New subject: [Maria-developers] 74b2eba1ca6: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING

Hi Serg! On Tue, Mar 26, 2019 at 10:48 PM Sergei Golubchik wrote:

...

Hi, Aleksey!

On Mar 26, Aleksey Midenkov wrote:

...
revision-id: 74b2eba1ca6 (mariadb-10.3.12-115-g74b2eba1ca6) parent(s): 2c0901d808b author: Aleksey Midenkov committer: Sergei Golubchik timestamp: 2019-03-26 17:43:38 +0100 message:

MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING

* HA_EXTRA_REMEMBER_POS, HA_EXTRA_RESTORE_POS for HEAP storage engine * Versioning tests support

diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index abe4254db3e..1d51947cc86 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -1643,7 +1643,12 @@ int vers_insert_history_row(TABLE *table) if (row_start->cmp(row_start->ptr, row_end->ptr) >= 0) return 0;

- return table->file->ha_write_row(table->record[0]); + int res; + if ((res= table->file->extra(HA_EXTRA_REMEMBER_POS))) + return res; + if ((res= table->file->ha_write_row(table->record[0]))) + return res; + return table->file->extra(HA_EXTRA_RESTORE_POS); }

Frankly, I don't like it. I mean, this particular fix is fine, but here's the problem:

We now have (at least) three features where the server might need to do something (insert/update/search) during the index/rnd scan.

And three different solutions: you use HA_EXTRA_REMEMBER_POS in system versioning, Nikita simply sets delete_while_scanning=false in application time period tables, and Sachin uses handler::clone() to create a second handler that can be used to check for hash collisions without disrupting the scan on the primary handler.

Actually I liked the Aleksey approach it is much easier then cloning and I don't have to free handler in close table Although I tried with long unique update I was getting assert fail in ha_index_read_map DBUG_ASSERT(inited==INDEX); Can we do (different )index scan after table->file->extra(HA_EXTRA_REMEMBER_POS) ?

...

I think it's getting somewhat out of hands. Can we please reduce the number of approaches to fix the same issue?

Regards, Sergei Chief Architect MariaDB and security@mariadb.org

_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp

1947

Age (days ago)

1950

Last active (days ago)

List overview

Download

4 comments

4 participants

participants (4)

Aleksey Midenkov
Nikita Malyavin
sachin setiya
Sergei Golubchik