Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: lvasiliev <lvasiliev@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2 5/5] iproto: Update error MsgPack encoding
Date: Thu, 16 Apr 2020 23:06:05 +0200	[thread overview]
Message-ID: <70497401-21cc-92ad-f596-38af99c3f484@tarantool.org> (raw)
In-Reply-To: <7fcb54c9-0fc3-65b8-4c32-3370e811debe@tarantool.org>

Thanks for the answers!

>>>> 7. Why are these values in a enum? I thought we
>>>> decided to use string types?
>>> Now we compare a string with error type when encode and use number further. If we encode error type as string we still have to compare the strings when decoding to create the error of the desired type. But number takes up less space versus string  when transmitting over a network and makes it possible to use a switch when create an error after decoding (which looks much nicer).
>>
>> Wait, so this is inside the MessagePack too? This is not a good
>> idea really. I thought you just converted strings to enums and
>> then enums to classes.
> Yes. It's not inside the MessagePack. MP_ERROR decodes by
> external handler as we discussed. It registered by
> luamp_set_decode_extension().

I meant it is inside MessagePack protocol, not library.

>> First about 'less space'. These are error objects. It does not
>> really matter if every one of them becomes +10 or even +20 bytes
>> if we use string types instead of numbers. They are not critical
>> in any aspect, not in perf nor in space.
>>
>> Second about switch, 'which looks nicer'. Nicely formatted if-else
>> sequence also look fine. This is subjective though. I won't argue.
>>
>> Thirdly, about why I think string types are better. The problem is
>> that I don't want us to document all the error types as numbers
>> and support them forever. When we use strings, we easily can remove
>> old errors, add new errors. When we use numbers, we
>>
>>      1) will need to document what every number means. Strings are
>>      self-documenting. OutOfMemory means out of memory, obviously.
>>      And so on.
> Ok.
>>
>>      2) will one day have holes in these numbers left from removed
>>      errors, this won't look nice, trust me.
> Ok.
>>
>>      3) that complicates compatibility. What if some error type was
>>      added to a newer tarantool version, and an old connector connected
>>      to the instance? How will it handle the new error types? With
>>      string types the problem does not exist.
> If I understand you correctly, you will have the same problem
> with string, because you can't create an error of unknown type.

String is the type. Client's connector don't need to keep a track of
all existing error types, and validate every incoming string. The
connector just need to be able to decode MP_STR, and that is all.

When you have numbers, client can't tell error type, if a new unknown
number is received. Instead of a nice human readable error type it
will be just an abstract number like '252'.

With string types whatever new errors are added in newer versions,
all client connectors will be able to decode them without necessity
to validate.

  reply	other threads:[~2020-04-16 21:06 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-10  8:10 [Tarantool-patches] [PATCH v2 0/5] Extending error functionality Leonid Vasiliev
2020-04-10  8:10 ` [Tarantool-patches] [PATCH v2 1/5] error: Add a Lua backtrace to error Leonid Vasiliev
2020-04-14  1:11   ` Vladislav Shpilevoy
2020-04-15  9:25     ` lvasiliev
2020-04-16  0:00       ` Vladislav Shpilevoy
2020-04-16  1:11         ` Alexander Turenko
2020-04-16  8:58           ` lvasiliev
2020-04-16  9:30             ` Alexander Turenko
2020-04-16 12:27               ` lvasiliev
2020-04-16 12:45                 ` Alexander Turenko
2020-04-16 17:48                   ` lvasiliev
2020-04-16  8:40         ` lvasiliev
2020-04-16  9:04           ` lvasiliev
2020-04-10  8:10 ` [Tarantool-patches] [PATCH v2 2/5] error: Add the custom error type Leonid Vasiliev
2020-04-14  1:11   ` Vladislav Shpilevoy
2020-04-15  9:25     ` lvasiliev
2020-04-16  0:02       ` Vladislav Shpilevoy
2020-04-16  9:18         ` lvasiliev
2020-04-16 21:03           ` Vladislav Shpilevoy
2020-04-10  8:10 ` [Tarantool-patches] [PATCH v2 3/5] error: Increase the number of fields transmitted through IPROTO Leonid Vasiliev
2020-04-14  1:12   ` Vladislav Shpilevoy
2020-04-15  9:25     ` lvasiliev
2020-04-16  0:02       ` Vladislav Shpilevoy
2020-04-10  8:10 ` [Tarantool-patches] [PATCH v2 4/5] iproto: Add session settings for IPROTO Leonid Vasiliev
2020-04-14  1:12   ` Vladislav Shpilevoy
2020-04-15  9:26     ` lvasiliev
2020-04-16  0:06       ` Vladislav Shpilevoy
2020-04-16  9:36         ` lvasiliev
2020-04-16 21:04           ` Vladislav Shpilevoy
2020-04-10  8:10 ` [Tarantool-patches] [PATCH v2 5/5] iproto: Update error MsgPack encoding Leonid Vasiliev
2020-04-14  1:12   ` Vladislav Shpilevoy
2020-04-15  9:25     ` lvasiliev
2020-04-16  0:11       ` Vladislav Shpilevoy
2020-04-16 10:04         ` lvasiliev
2020-04-16 21:06           ` Vladislav Shpilevoy [this message]
2020-04-14  1:10 ` [Tarantool-patches] [PATCH v2 0/5] Extending error functionality Vladislav Shpilevoy
2020-04-15  9:48   ` lvasiliev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=70497401-21cc-92ad-f596-38af99c3f484@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=lvasiliev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 5/5] iproto: Update error MsgPack encoding' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox