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.

Sign up for GitHub

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

Jump to bottom

msgpackffi does not respect msgpack.cfg options #4499

Open
Gerold103 opened this issue Sep 13, 2019 · 1 comment
Open

msgpackffi does not respect msgpack.cfg options #4499

Gerold103 opened this issue Sep 13, 2019 · 1 comment
Labels
app blocked Not ready to be implemented bug Something isn't working

Comments

@Gerold103
Copy link
Collaborator

#4434 fixes it for encode_max_depth, but other options are still ignored. It should be either fixed or documented. I think, a fix is better.
@Gerold103 Gerold103 added bug Something isn't working app good first issue Good for newcomers labels Sep 13, 2019
@MariaHajdic MariaHajdic self-assigned this Sep 20, 2019
MariaHajdic added a commit that referenced this issue Sep 25, 2019
@MariaHajdic
Msgpackffi considers msgpack.cfg options now
bcc297d 
Lua serializers have common configuration options
that mostly were not respected in msgpackffi module.
This patch fixes that for several configurations:
encode_use_tostring, encode_invalid_as_nil and
encode_load_metatables.
Options encode/decode_invalid_numbers are suggested
to not be considered in msgpackffi module since lua
processed them properly without this check so adding
it would mean cutting functionality provided by the
language in case this field was set to be false.

Closes #4499
MariaHajdic added a commit that referenced this issue Sep 25, 2019
@MariaHajdic
Msgpackffi considers msgpack.cfg options now
32818ef 
Lua serializers have common configuration options
that mostly were not respected in msgpackffi module.
This patch fixes that for several configurations:
encode_use_tostring, encode_invalid_as_nil and
encode_load_metatables.
Options encode/decode_invalid_numbers are suggested
to not be considered in msgpackffi module since lua
processed them properly without this check so adding
it would mean cutting functionality provided by the
language in case this field was set to be false.

Closes #4499
MariaHajdic added a commit that referenced this issue Sep 25, 2019
@MariaHajdic
Merge branch 'eljashm/gh-4499-msgpackffi-ignores-msgpack.cfg-options'…
faa0d57 
… of https://github.com/tarantool/tarantool into eljashm/gh-4499-msgpackffi-ignores-msgpack.cfg-options
@kyukhin kyukhin added this to the 2.2.2 milestone Sep 26, 2019
MariaHajdic added a commit that referenced this issue Nov 6, 2019
@MariaHajdic
Msgpackffi considers msgpack.cfg options now
7efdedd 
Lua serializers have common configuration options
that mostly were not respected in msgpackffi module.
This patch fixes that for several configurations:
encode_use_tostring, encode_invalid_as_nil and
encode_load_metatables.
Options encode/decode_invalid_numbers are suggested
to not be considered in msgpackffi module since lua
processed them properly without this check so adding
it would mean cutting functionality provided by the
language in case this field was set to be false.

Closes #4499
@Totktonada Totktonada mentioned this issue Nov 11, 2019
Closed
MariaHajdic added a commit that referenced this issue Nov 20, 2019
@MariaHajdic
Msgpackffi considers msgpack.cfg options now
ea836ec 
Lua serializers have common configuration options
that mostly were not respected in msgpackffi module.
This patch fixes that for several configurations:
encode_use_tostring, encode_invalid_as_nil and
encode_load_metatables.
Options encode/decode_invalid_numbers are suggested
to not be considered in msgpackffi module since lua
processed them properly without this check so adding
it would mean cutting functionality provided by the
language in case this field was set to be false.

Closes #4499
MariaHajdic added a commit that referenced this issue Nov 20, 2019
@MariaHajdic
Msgpackffi considers msgpack.cfg options now
0ad4be5 
Lua serializers have common configuration options
that mostly were not respected in msgpackffi module.
This patch fixes that for several configurations:
encode_use_tostring, encode_invalid_as_nil and
encode_load_metatables.
Options encode/decode_invalid_numbers are suggested
to not be considered in msgpackffi module since lua
processed them properly without this check so adding
it would mean cutting functionality provided by the
language in case this field was set to be false.

Closes #4499
MariaHajdic added a commit that referenced this issue Nov 25, 2019
@MariaHajdic
Msgpackffi considers msgpack.cfg options now
4502f7e 
Lua serializers have common configuration options
that mostly were not respected in msgpackffi module.
This patch fixes that for several configurations:
encode_use_tostring, encode_invalid_as_nil and
encode_load_metatables.
Options encode/decode_invalid_numbers are suggested
to not be considered in msgpackffi module since lua
processed them properly without this check so adding
it would mean cutting functionality provided by the
language in case this field was set to be false.

Closes #4499
MariaHajdic added a commit that referenced this issue Nov 29, 2019
@MariaHajdic
Msgpackffi considers msgpack.cfg options now
065bdd4 
Lua serializers have common configuration options
that mostly were not respected in msgpackffi module.
This patch fixes that for several configurations:
encode_use_tostring, encode_invalid_as_nil and
encode_load_metatables.
Options encode/decode_invalid_numbers are suggested
to not be considered in msgpackffi module since lua
processed them properly without this check so adding
it would mean cutting functionality provided by the
language in case this field was set to be false.

Closes #4499
MariaHajdic added a commit that referenced this issue Nov 29, 2019
@MariaHajdic
Msgpackffi considers msgpack.cfg options now
d440c35 
Lua serializers have common configuration options
that mostly were not respected in msgpackffi module.
This patch fixes that for several configurations:
encode_use_tostring, encode_invalid_as_nil and
encode_load_metatables.
Options encode/decode_invalid_numbers are suggested
to not be considered in msgpackffi module since lua
processed them properly without this check so adding
it would mean cutting functionality provided by the
language in case this field was set to be false.

Closes #4499
MariaHajdic added a commit that referenced this issue Nov 29, 2019
@MariaHajdic
Msgpackffi considers msgpack.cfg options now
8203dea 
Msgpack.cfg options changes did not affect msgpackffi serializer state.
This patch fixes it for options encode_load_metatables, decode_ and
encode_invalid_numbers, encode_use_tostring and encode_invalid_as_nil.

Closes #4499
MariaHajdic added a commit that referenced this issue Dec 24, 2019
@MariaHajdic
Msgpackffi considers msgpack.cfg options now
b0a2478 
Msgpack.cfg options changes did not affect msgpackffi serializer state.
This patch fixes it for options encode_load_metatables, decode_ and
encode_invalid_numbers, encode_use_tostring and encode_invalid_as_nil.

Closes #4499
@kyukhin kyukhin modified the milestones: 2.2.2, 2.2.3 Dec 30, 2019
MariaHajdic added a commit that referenced this issue Jan 18, 2020
@MariaHajdic
Msgpackffi considers msgpack.cfg options now
5f29768 
Msgpack.cfg options changes did not affect msgpackffi serializer state.
This patch fixes it for options encode_load_metatables, decode_ and
encode_invalid_numbers, encode_use_tostring and encode_invalid_as_nil.

Closes #4499
MariaHajdic added a commit that referenced this issue Jan 24, 2020
@MariaHajdic
lua: msgpackffi considers msgpack.cfg options now
0b609fb 
Msgpack.cfg reconfiguration did not affect ffi module behavior. This
patch fixes it for all the options that were not yet covered elsewhere:
encode_sparse_convert, encode_sparse_ratio, encode_sparse_safe,
encode_invalid_numbers & decode_invalid_numbers,
encode_load_metatables & decode_save_metatables,
encode_use_tostring, encode_invalid_as_nil.

Most of common serializers' test were not applicable to msgpackffi since
it has no cfg field of its own. This issue was also solved.

Closes #4499
MariaHajdic added a commit that referenced this issue Jan 24, 2020
@MariaHajdic
lua: msgpackffi considers msgpack.cfg options now
d7df35c 
Msgpack.cfg reconfiguration did not affect ffi module behavior. This
patch fixes it for all the options that were not yet covered elsewhere:
encode_sparse_convert, encode_sparse_ratio, encode_sparse_safe,
encode_invalid_numbers & decode_invalid_numbers,
encode_load_metatables & decode_save_metatables,
encode_use_tostring, encode_invalid_as_nil.

Most of common serializers' test were not applicable to msgpackffi since
it has no cfg field of its own. This issue was also solved.

Closes #4499
MariaHajdic added a commit that referenced this issue Jan 27, 2020
@MariaHajdic
lua: msgpackffi considers msgpack.cfg options now
c2bb399 
Msgpack.cfg reconfiguration did not affect ffi module behavior. This
patch fixes it for all the options that were not yet covered elsewhere:
encode_sparse_convert, encode_sparse_ratio, encode_sparse_safe,
encode_invalid_numbers & decode_invalid_numbers,
encode_load_metatables & decode_save_metatables,
encode_use_tostring, encode_invalid_as_nil.

Most of common serializers' test were not applicable to msgpackffi since
it has no cfg field of its own. This issue was also solved.

Closes #4499
@kyukhin kyukhin modified the milestones: 2.2.3, 2.3.3 Apr 20, 2020
@kyukhin kyukhin modified the milestones: 2.3.3, wishlist Jun 22, 2020
@Totktonada
Copy link
Member

I propose to consider this issue as blocked and solve the following questions first:

  • Decide, whether we should support msgpack and msgpackffi both (in the scope of msgpackffi is x3 times slower than Lua C #5896, after thorough performance testing of current state of the modules and proof that the observed msgpackffi slowness is not result of a degradation after introducing some configuration option, a tarantool specific extension type support or another change).
  • If we'll decide to support both modules, decide whether we'll aim to have the same API two different ones: a bit slower msgpack with full API and faster msgpackffi with a subset of the API (we can decide in the scope of msgpackffi is x3 times slower than Lua C #5896 too).

@Totktonada Totktonada added blocked Not ready to be implemented and removed good first issue Good for newcomers labels Jul 20, 2021
locker added a commit to locker/tarantool that referenced this issue Sep 14, 2021
@locker
lua/msgpack: enable encoding errors as msgpack extension
a64b5b9 
This patch adds a new msgpack.cfg: encode_error_as_ext. Setting it makes
msgpack and msgpackffi modules encode errors as the MP_ERROR msgpack
extension. If the flag is unset, msgpack.encode behavior depends on
encode_load_metatables, encode_use_tostring, and encode_invalid_as_nil
options, see luaL_convertfield(), while msgpackffi.encode() will always
encode errors as strings. The latter needs to be fixed, but it's out of
the scope of this work and tracked separately, see tarantool#4499.

The new option is enabled by default.

Interaction with box.session.settings.error_marshaling_enabled: errors
are encoded as the MP_ERROR msgpack extension when returned via IPROTO
iff both error_marshaling_enabled and encode_error_as_ext are set.

Closes tarantool#6433

@TarantoolBot document
Title: Document msgpack.cfg.encode_error_as_ext

The new option determines how error objects (see box.error.new) are
encoded in the msgpack format:
 - If it's set, errors are encoded as the MP_ERROR msgpack extension.
   This is the default behavior.
 - If it's unset, the encoded format depends on other msgpack
   configuration options (encode_load_metatables, encode_use_tostring,
   encode_invalid_as_nil). With the otherwise default configuration,
   they are encoded as strings (see error.message).

Functions affected by the default configuration (msgpack.cfg):
 - msgpack and msgpackffi modules
 - Storing errors in tuples and spaces (box.tuple.new)
 - Returning errors from IPROTO CALL/EVAL
locker added a commit to locker/tarantool that referenced this issue Sep 14, 2021
@locker
lua/msgpack: enable encoding errors as msgpack extension
7ade4c0 
This patch adds a new msgpack.cfg: encode_error_as_ext. Setting it makes
msgpack and msgpackffi modules encode errors as the MP_ERROR msgpack
extension. If the flag is unset, msgpack.encode behavior depends on
encode_load_metatables, encode_use_tostring, and encode_invalid_as_nil
options, see luaL_convertfield(), while msgpackffi.encode() will always
encode errors as strings. The latter needs to be fixed, but it's out of
the scope of this work and tracked separately, see tarantool#4499.

The new option is enabled by default.

Interaction with box.session.settings.error_marshaling_enabled: errors
are encoded as the MP_ERROR msgpack extension when returned via IPROTO
iff both error_marshaling_enabled and encode_error_as_ext are set.

Closes tarantool#6433

@TarantoolBot document
Title: Document msgpack.cfg.encode_error_as_ext

The new option determines how error objects (see box.error.new) are
encoded in the msgpack format:
 - If it's set, errors are encoded as the MP_ERROR msgpack extension.
   This is the default behavior.
 - If it's unset, the encoded format depends on other msgpack
   configuration options (encode_load_metatables, encode_use_tostring,
   encode_invalid_as_nil). With the otherwise default configuration,
   they are encoded as strings (see error.message).

Functions affected by the default configuration (msgpack.cfg):
 - msgpack and msgpackffi modules
 - Storing errors in tuples and spaces (box.tuple.new)
 - Returning errors from IPROTO CALL/EVAL
locker added a commit to locker/tarantool that referenced this issue Sep 15, 2021
@locker
lua/msgpack: enable encoding errors as msgpack extension
cfc36f3 
This patch adds a new msgpack.cfg: encode_error_as_ext. Setting it makes
msgpack and msgpackffi modules encode errors as the MP_ERROR msgpack
extension. If the flag is unset, msgpack.encode behavior depends on
encode_load_metatables, encode_use_tostring, and encode_invalid_as_nil
options, see luaL_convertfield(), while msgpackffi.encode() will always
encode errors as strings. The latter needs to be fixed, but it's out of
the scope of this work and tracked separately, see tarantool#4499.

The new option is enabled by default.

Interaction with box.session.settings.error_marshaling_enabled: errors
are encoded as the MP_ERROR msgpack extension when returned via IPROTO
iff both error_marshaling_enabled and encode_error_as_ext are set.

Closes tarantool#6433

@TarantoolBot document
Title: Document msgpack.cfg.encode_error_as_ext

The new option determines how error objects (see box.error.new) are
encoded in the msgpack format:
 - If it's set, errors are encoded as the MP_ERROR msgpack extension.
   This is the default behavior.
 - If it's unset, the encoded format depends on other msgpack
   configuration options (encode_load_metatables, encode_use_tostring,
   encode_invalid_as_nil). With the otherwise default configuration,
   they are encoded as strings (see error.message).

Functions affected by the default configuration (msgpack.cfg):
 - msgpack and msgpackffi modules
 - Storing errors in tuples and spaces (box.tuple.new)
 - Returning errors from IPROTO CALL/EVAL
locker added a commit to locker/tarantool that referenced this issue Sep 16, 2021
@locker
lua/msgpack: enable encoding errors as msgpack extension
a2dce10 
This patch adds a new msgpack.cfg: encode_error_as_ext. Setting it makes
msgpack and msgpackffi modules encode errors as the MP_ERROR msgpack
extension. If the flag is unset, msgpack.encode behavior depends on
encode_load_metatables, encode_use_tostring, and encode_invalid_as_nil
options, see luaL_convertfield(), while msgpackffi.encode() will always
encode errors as strings. The latter needs to be fixed, but it's out of
the scope of this work and tracked separately, see tarantool#4499.

The new option is enabled by default.

Interaction with box.session.settings.error_marshaling_enabled: errors
are encoded as the MP_ERROR msgpack extension when returned via IPROTO
iff both error_marshaling_enabled and encode_error_as_ext are set.

Closes tarantool#6433

@TarantoolBot document
Title: Document msgpack.cfg.encode_error_as_ext

The new option determines how error objects (see box.error.new) are
encoded in the msgpack format:
 - If it's set, errors are encoded as the MP_ERROR msgpack extension.
   This is the default behavior.
 - If it's unset, the encoded format depends on other msgpack
   configuration options (encode_load_metatables, encode_use_tostring,
   encode_invalid_as_nil). With the otherwise default configuration,
   they are encoded as strings (see error.message).

Functions affected by the default configuration (msgpack.cfg):
 - msgpack and msgpackffi modules
 - Storing errors in tuples and spaces (box.tuple.new)
 - Returning errors from IPROTO CALL/EVAL
locker added a commit to locker/tarantool that referenced this issue Sep 16, 2021
@locker
lua/msgpack: enable encoding errors as msgpack extension
ef906d7 
This patch adds a new msgpack.cfg: encode_error_as_ext. Setting it makes
msgpack and msgpackffi modules encode errors as the MP_ERROR msgpack
extension. If the flag is unset, msgpack.encode behavior depends on
encode_load_metatables, encode_use_tostring, and encode_invalid_as_nil
options, see luaL_convertfield(), while msgpackffi.encode() will always
encode errors as strings. The latter needs to be fixed, but it's out of
the scope of this work and tracked separately, see tarantool#4499.

The new option is enabled by default.

Interaction with box.session.settings.error_marshaling_enabled: errors
are encoded as the MP_ERROR msgpack extension when returned via IPROTO
iff both error_marshaling_enabled and encode_error_as_ext are set.

Closes tarantool#6433

@TarantoolBot document
Title: Document msgpack.cfg.encode_error_as_ext

The new option determines how error objects (see box.error.new) are
encoded in the msgpack format:
 - If it's set, errors are encoded as the MP_ERROR msgpack extension.
   This is the default behavior.
 - If it's unset, the encoded format depends on other msgpack
   configuration options (encode_load_metatables, encode_use_tostring,
   encode_invalid_as_nil). With the otherwise default configuration,
   they are encoded as strings (see error.message).

Functions affected by the default configuration (msgpack.cfg):
 - msgpack and msgpackffi modules
 - Storing errors in tuples and spaces (box.tuple.new)
 - Returning errors from IPROTO CALL/EVAL
locker added a commit to locker/tarantool that referenced this issue Sep 17, 2021
@locker
lua/msgpack: enable encoding errors as msgpack extension
cd89e56 
This patch adds a new msgpack.cfg: encode_error_as_ext. Setting it makes
msgpack and msgpackffi modules encode errors as the MP_ERROR msgpack
extension. If the flag is unset, msgpack.encode behavior depends on
encode_load_metatables, encode_use_tostring, and encode_invalid_as_nil
options, see luaL_convertfield(), while msgpackffi.encode() will always
encode errors as strings. The latter needs to be fixed, but it's out of
the scope of this work and tracked separately, see tarantool#4499.

The new option is enabled by default.

Interaction with box.session.settings.error_marshaling_enabled: errors
are encoded as the MP_ERROR msgpack extension when returned via IPROTO
iff both error_marshaling_enabled and encode_error_as_ext are set.

Closes tarantool#6433

@TarantoolBot document
Title: Document msgpack.cfg.encode_error_as_ext

The new option determines how error objects (see box.error.new) are
encoded in the msgpack format:
 - If it's set, errors are encoded as the MP_ERROR msgpack extension.
   This is the default behavior.
 - If it's unset, the encoded format depends on other msgpack
   configuration options (encode_load_metatables, encode_use_tostring,
   encode_invalid_as_nil). With the otherwise default configuration,
   they are encoded as strings (see error.message).

Functions affected by the default configuration (msgpack.cfg):
 - msgpack and msgpackffi modules
 - Storing errors in tuples and spaces (box.tuple.new)
 - Returning errors from IPROTO CALL/EVAL
locker added a commit that referenced this issue Sep 17, 2021
@locker
lua/msgpack: enable encoding errors as msgpack extension
fa1652d 
This patch adds a new msgpack.cfg: encode_error_as_ext. Setting it makes
msgpack and msgpackffi modules encode errors as the MP_ERROR msgpack
extension. If the flag is unset, msgpack.encode behavior depends on
encode_load_metatables, encode_use_tostring, and encode_invalid_as_nil
options, see luaL_convertfield(), while msgpackffi.encode() will always
encode errors as strings. The latter needs to be fixed, but it's out of
the scope of this work and tracked separately, see #4499.

The new option is enabled by default.

Interaction with box.session.settings.error_marshaling_enabled: errors
are encoded as the MP_ERROR msgpack extension when returned via IPROTO
iff both error_marshaling_enabled and encode_error_as_ext are set.

Closes #6433

@TarantoolBot document
Title: Document msgpack.cfg.encode_error_as_ext

The new option determines how error objects (see box.error.new) are
encoded in the msgpack format:
 - If it's set, errors are encoded as the MP_ERROR msgpack extension.
   This is the default behavior.
 - If it's unset, the encoded format depends on other msgpack
   configuration options (encode_load_metatables, encode_use_tostring,
   encode_invalid_as_nil). With the otherwise default configuration,
   they are encoded as strings (see error.message).

Functions affected by the default configuration (msgpack.cfg):
 - msgpack and msgpackffi modules
 - Storing errors in tuples and spaces (box.tuple.new)
 - Returning errors from IPROTO CALL/EVAL
@kyukhin kyukhin removed this from the wishlist milestone Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
app blocked Not ready to be implemented bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Totktonada @Gerold103 @kyukhin @MariaHajdic