Hello Sergei!
On Fri, 29 Sept 2023 at 23:14, Sergei Golubchik
But I suggest to try to swap the implementation. Make
online_alter_hton->savepoint_offset=sizeof(void*);
and in savepoint_set create a list or an array of all offsets and store it in the savepoint. Something like this:
struct { uint savepoints; my_off_t log_prev_pos[1]; } oasv;
int online_alter_savepoint_set(handlerton *hton, THD *thd, oasv **sv) { uint count= 0, i=0; for (auto &cache: cache_list) count++; DBUG_ASSERT(count);
*sv= malloc(sizeof(oasv) + sizeof(my_off_t)*(count - 1)); (*sv)->savepoints= count;
for (auto &cache: cache_list) (*sv)->log_prev_pos[i]= cache.get_byte_position(); }
and so on. You just need to make sure you always use push_back and not push_front for adding caches to the cache_list.
instead of a structure with a counter you can use an array with MY_OFF_T_UNDEF (not 0!) at the end.
Just checked, I can really make it zero yes! Nice idea of exposing a flexible array member to reduce a number of allocations. I'll remark also, that my implementation can also be alloc-optimized. Overall I don't like the savepoint_offset design. Even Innodb only uses it to store a local counter, which I also had in an early implementation (but then found that a pointer itself can identify it). Only binlog is lucky to use it on purpose. Also if a plugin is uninstalled, the offset memory will never be reused. And if a dynamic size is required for a savepoint, like we have, it will not be automatically deallocated. So in your case, we'd also need to deallocate all the skipped savepoints somehow. Probably, they should be linked in a list, but then it's not worth it to transponse the implementation. -- Yours truly, Nikita Malyavin