maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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/4eb8fd9c211f39669417b46197a6a6f1e8bf7ec2

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