Hi, Piotr! On Aug 25, Piotr Jurkiewicz wrote:

Hi again,

there are some issues which I think needs to be resolved before merging the code.

1. Bundle libevent or not?

Memcached daemon is built with the help of libevent library. MySQL decided to bundle this library inside its sources, in 'libevent' top level directory. This allows to compile and run the plugin on machines without installed system-wide libevent library.

The bundled libevent is used by default during the compilation of plugin. Optionally, it can be specified in CMake variable to use system-wide libevent instead.

In my experience distributions really really hate it when we bundle libraries. I mean, when we don't use system libraries. So, bundling is fine as long as there's an option to use system library - and all our packages will always do that. But in that case I feel like it's kind of silly to add a third-party library to our source tree and not use it. So I'd prefer not to bundle it at all. The only use case when it might be needed - our binary tarballs. There we'd prefer not to have a dependency on system libevent. But this can be easily solved by linking with static system libevent.a on our build hosts. We already do that for some other libraries. So, either way, there's no need to bundle libevent.

It needs to be decided whether MariaDB should bundle libevent as well inside its sources. Possible outcomes:

a) Leave bundled libevent as is, in the 'libevent' top level directory. b) Move bundled libevent into the 'daemon_memcached' plugin directory, in order to not clutter the top level directory. c) Remove bundled libevent. After that, system libevent headers will become compile-time dependency and system libevent shared library runtime dependency of the plugin.

c) is good

2. How to transfer InnoDB API callback array pointer to the plugin?

InnoDB Memcached interface engine needs to obtain the pointer to InnoDB API callback array in some way. MySQL originally implemented that in 'sql/sql_plugin.cc:plugin_initialize()' in the following way:

https://github.com/piotrjurkiewicz/mariadb-server/commit/4eb8fd9c211f3966941...

Yuck! That's pretty awful. Although consistent with how they do plugin things at MySQL (see e.g. validate_plugin in sql_acl.cc). As far as plugin initialization order is concerned, MariaDB has well defined plugin initialization order by type. We can have all daemon plugins to be initialized after all storage engines, if needed. This isn't perfect (not enough fine-grained to my taste), but it'll do for this task. If you'd like you can try to come up with a better way of doing per-plugin initializion ordering. As for innodb_api_cb, two thoughts: 1. there is no "data" in MariaDB handlertons. So that code will not work at all, thanks god :) 2. one plugin wants to talk directly to another plugin - okay, fine, they can do it, but please don't make the server to help them; if the server is not involved in this inter-plugin communication, it should be completely left out of it. Practically it means that daemon_memcached plugin should do simply dlsym("innodb_api_cb"). Or it can use innodb_api_cb directly and let dynamic linker take care of that.

As you can see, this is not a clean solution.

Possible outcomes:

a) Leave the MySQL's solution as is.

nope

b) Figure out a better way to transfer the pointer.

see above

3. Where should I post the documentation?

knowledge base. but if you're going to do it now, don't forget a disclaimer that "this feature is not pushed yet, blablabla, not in any release, whatever". This usually goes into <<style class="redbox">>...<</style>>.

4. How should I prepare the pull request? Should I rebase my branch on the top of current branch before that?

That would be good, yes. Although first someone will review the pull request, and only after it'll be "ok to push", then the pull request should be rebased on top of the current branch. There's no need to try to keep it constantly rebased, it'd be a waste of time. Did you have to modify a lot of code in the server (outside of plugin/daemon_memcached) ? Regards, Sergei