Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: Kirill Shcherbatov <kshcherbatov@tarantool.org>,
	tarantool-patches@freelists.org, georgy@tarantool.org
Cc: alexander.turenko@tarantool.org
Subject: [tarantool-patches] Re: [PATCH v3 1/6] box: rfc for stacked diagnostic area in Tarantool
Date: Mon, 9 Sep 2019 21:44:17 +0200	[thread overview]
Message-ID: <82acb857-925d-fd47-2cae-3c8592c7eb26@tarantool.org> (raw)
In-Reply-To: <7b5287fa-c430-ca96-3b5b-0b879f28fba9@tarantool.org>

> 2.
>>> +```
>>> +
>>> +Error cdata object has `:unpack()`, `:raise()`, `:match(...)`,
>>> `:__serialize()` methods and uniform `.type`, `.message` and `.trace`
>>> fields. +
>>> +
>> What if we want to throw a table error - as vshard does.
> 
> https://github.com/tarantool/vshard/blob/master/vshard/error.lua
> As fores I've understood, vshard has an own error representation same as box_error
> adapter that constructs vshard-compatible table error object.

VShard repeats error:unpack() format. If you change result of unpack(), then
I will update VShard (if that will be necessary).

> The backward compatibility wouldn't be broken with introducing a couple new fields in core error object.

Yes, if you only add new fields to error:unpack(), then nothing
will break.

> However, to have a profit with new stacked diagnostics functionality the box_error adapter should be
> updated correspondingly. This likely requires extending vshard error functionality. 
> I'd better create a related ticket for vshard module, if you are agree with me.
> 
> 
> 3.
>> I don't see any valid purpose for svp* methods. Please explain the cases when 
>> we need to rollback a diag area.

Me neither. Svp won't solve any problems IMO. It will
just complicate the code. Talking of cases below - if you
call a function, you need to know how it works. If you called
a function not setting a diag, and you expected a diag, that
you either didn't test your code properly, or you was
inattentive, or both. Svp won't solve neither of that problems.
You still may forget to reset svp, to set svp, to test the code.

> Kostya had a strong opinion about an ability to reset a last-errors set.

Honestly, I didn't understand his point. And I think, if
we ever will understand, we can easily add svps. But if we add
them now and they appear to be useless, then we won't be able
to drop them, because of compatibility.

> 5.
>>> +-- Return a reason error object for given error
>>> +-- (when exists, nil otherwise)
>>> +box.error.prev(error) == error.prev
>>> +```
>> You let out of scope standard lua "error()" call. Please take it into 
>> consideration.
>  
> I can't say nothing about error() call. It is not box error, it can't be extended.
> It should be deprecated when #4398 would be closed, right?

error() is a built-in Lua function, it is a part of the language. You can't
deprecate it. But I also don't understand what is wrong with it. Seems like
it will work just like always.

  reply	other threads:[~2019-09-09 19:40 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-02 14:51 [tarantool-patches] [PATCH v3 0/6] box: stacked diagnostics area in fiber Kirill Shcherbatov
2019-09-02 14:51 ` [tarantool-patches] [PATCH v3 1/6] box: rfc for stacked diagnostic area in Tarantool Kirill Shcherbatov
2019-09-09  8:13   ` [tarantool-patches] " Kirill Shcherbatov
2019-09-09 19:44     ` Vladislav Shpilevoy [this message]
2019-09-09 19:44   ` Vladislav Shpilevoy
2019-09-02 14:51 ` [tarantool-patches] [PATCH v3 2/6] box: rename diag_add_error to diag_set_error Kirill Shcherbatov
2019-09-02 14:51 ` [tarantool-patches] [PATCH v3 3/6] box: introduce stacked diagnostic area Kirill Shcherbatov
2019-09-02 14:51 ` [tarantool-patches] [PATCH v3 4/6] box: extend box.error.new({}) API Kirill Shcherbatov
2019-09-02 14:51 ` [tarantool-patches] [PATCH v3 5/6] iproto: refactor error encoding with mpstream Kirill Shcherbatov
2019-09-04 10:53   ` [tarantool-patches] " Konstantin Osipov
2019-09-02 14:51 ` [tarantool-patches] [PATCH v3 6/6] iproto: support stacked diagnostics for errors Kirill Shcherbatov

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=82acb857-925d-fd47-2cae-3c8592c7eb26@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=georgy@tarantool.org \
    --cc=kshcherbatov@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH v3 1/6] box: rfc for stacked diagnostic area in Tarantool' \
    /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