Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump small library version #106

Closed
wants to merge 1 commit into from
Closed

Conversation

ligurio
Copy link
Member

@ligurio ligurio commented Feb 16, 2022

Current version of Small library made on Jan 2020 - commit "Revert "Free
all slabs on region reset" (3df5050171d040bfe5717b4bddf57da3b312ffe4).
Since Jan 2020 there were added many changes to Small library that
includes also regression bugfixes, see [1].

  1. https://github.com/tarantool/small/compare/3df5050171d040bfe5717b4bddf57da3b312ffe4..055458f1d45948f7d768e2499496926dcf78b26f

Closes #92

Current version of Small library made on Jan 2020 - commit "Revert "Free
all slabs on region reset" (3df5050171d040bfe5717b4bddf57da3b312ffe4).
Since Jan 2020 there were added many changes to Small library that
includes also regression bugfixes, see [1].

1. https://github.com/tarantool/small/compare/3df5050171d040bfe5717b4bddf57da3b312ffe4..055458f1d45948f7d768e2499496926dcf78b26f

Closes #92
@ligurio
Copy link
Member Author

ligurio commented Feb 16, 2022

Tests on 2.5, 2.6, 2.7, 2.8 failed.

@ligurio ligurio marked this pull request as draft February 16, 2022 18:03
Comment on lines +37 to +38
#include <include/small/ibuf.h>
#include <include/small/obuf.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is better to fix include_directories() in CMakeLists.txt.

@Totktonada
Copy link
Member

Since we use small here and in tarantool and some tarantool versions have open symbols aside of the export list (and so symbol name clash is possible, see #59), we can't just promote the small revision here. It'll break tarantool versions (not so old ones), where the symbols are open.

We can accurately backport commits that does not touch data layout of small's structures and don't rework function behaviour much. However I would go into another way.

My plan is to keep small version here for a while: so it will work on all actual versions of tarantool. After some graceful period (a year, maybe) or if we'll see a real customer problem, bump small here and release the new module version (and cleanly state, which tarantool versions are not supported anymore). I would also add a version check and report a good error if the module is run on improper tarantool version.

Anyway, we should confirm the analysis from #59 after PR #102 and understand all consequences of updating the small library in the module.

@Totktonada
Copy link
Member

Or better: fallback to malloc when we can't use small.

@Totktonada
Copy link
Member

Or better: fallback to malloc when we can't use small.

Maybe it is not so good option as looks on the first glance. We'll need to re-implement mempool, region, ibuf and obuf API on top of malloc, which looks as considerable amount of work.

@Totktonada
Copy link
Member

Simmarized possible options in #92 (comment).

In context of the patch: we should at least add the version check in memcached.create() to don't allow to create an instance on tarantool 2.5-2.8. And refrain ourself from releasing the patch until those tarantool versions will gone in the practical sense.

@ligurio ligurio closed this May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump version of small library
2 participants