[tarantool-patches] [PATCH 0/4] box/console: Improve lua mode for remote console

[tarantool-patches] [PATCH 0/4] box/console: Improve lua mode for remote console

[Cyrill Gorcunov]

This series address a problem where we can hang a connection setting up
lua mode while server continue treating all data as having built-in yaml
end of string terminator.

I've tested the series manually (and since we still use yaml as a default
output it should not break anything existing) but my next priority is
lifting up our test engine and use new require('console').eos() helper
inside to be able to read lua replies from server test jobs.

Thus I post the series asap just to gather feedback.

The following changes since commit 2d5e56fffe894c1fb77fe77bcf12b34da772b2a7:

  wal: make wal_sync fail on write error (2019-08-14 19:43:10 +0300)

are available in the Git repository at:

  https://github.com/cyrillos/tarantool.git console-fix-1

for you to fetch changes up to 396e9a5f6e3ec0fb03c6603ecaef59ee7073420c:

  box/console: Provide console.eos() api (2019-08-15 17:30:33 +0300)

----------------------------------------------------------------
Cyrill Gorcunov (4):
      box/console: Add explicit output EOS mapping
      box/console: Refactor command handling
      box/console: Fix hang in remote console lua mode
      box/console: Provide console.eos() api

 src/box/lua/console.lua | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 94 insertions(+), 13 deletions(-)
-- 
2.20.1

[tarantool-patches] [PATCH 1/4] box/console: Add explicit output EOS mapping

[Cyrill Gorcunov]

This will help us to distinguish end of string/stream
in text protocols (such as remote console connection).

Note that we start printing ";" terminator for every
lua output. Actually current yaml output does the
same but inside console.c module.

And since lua output is yet a new feature in stabilization
phase we're safe to make such changes without breaking api.

Part-of #3834
---
 src/box/lua/console.lua | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index 64086cf5b..be277d610 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -11,6 +11,7 @@ local urilib = require('uri')
 local yaml = require('yaml')
 local net_box = require('net.box')
 
+local LUA_TERM = ';'
 local YAML_TERM = '\n...\n'
 local PUSH_TAG_HANDLE = '!push!'
 
@@ -19,6 +20,10 @@ local PUSH_TAG_HANDLE = '!push!'
 -- compatibility reason.
 local default_output_format = { ["fmt"] = "yaml", ["opts"] = nil }
 local output_handlers = { }
+--
+-- End of streams/strings. They will allow to recognize blocks
+-- depending on output format.
+local output_eos = { ["yaml"] = YAML_TERM, ["lua"] = LUA_TERM }
 
 output_handlers["yaml"] = function(status, opts, ...)
     local err
@@ -42,9 +47,11 @@ end
 
 output_handlers["lua"] = function(status, opts, ...)
     --
-    -- Don't print nil if there is no data
+    -- If no data present at least EOS should be put,
+    -- otherwise wire readers won't be able to find
+    -- where end of string is.
     if not ... then
-        return ""
+        return output_eos["lua"]
     end
     --
     -- Map internal symbols which serpent doesn't know
@@ -64,9 +71,9 @@ output_handlers["lua"] = function(status, opts, ...)
         nocode = true,
     }
     if opts == "block" then
-        return serpent.block(..., serpent_opts)
+        return serpent.block(..., serpent_opts) .. output_eos["lua"]
     end
-    return serpent.line(..., serpent_opts)
+    return serpent.line(..., serpent_opts) .. output_eos["lua"]
 end
 
 local function output_verify_opts(fmt, opts)
-- 
2.20.1

[tarantool-patches] [PATCH 2/4] box/console: Refactor command handling

[Cyrill Gorcunov]

We will need to parse and verify "\set" commands
in remote console text-wire protocol thus to not
duplicate the code lets factor helper out for
reuse sake.

Part-of #3834
---
 src/box/lua/console.lua | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index be277d610..004625059 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -139,11 +139,16 @@ local function output_save(fmt, opts)
     }
 end
 
-local function format(status, ...)
+local function current_output()
     local d = box.session.storage.console_output_format
     if d == nil then
-        d = default_output_format
+        return default_output_format
     end
+    return d
+end
+
+local function format(status, ...)
+    local d = current_output()
     return output_handlers[d["fmt"]](status, d["opts"], ...)
 end
 
@@ -234,21 +239,40 @@ local operators = {
     q = quit
 }
 
-local function preprocess(storage, line)
+local function parse_operators(line)
     local items = {}
     for item in string.gmatch(line, '([^%s]+)') do
         items[#items + 1] = item
     end
     if #items == 0 then
-        return help_wrapper()
+        return 0, nil
     end
     if operators[items[1]] == nil then
+        return #items, nil
+    end
+    return #items, items
+end
+
+local function preprocess(storage, line)
+    local nr_items, items = parse_operators(line)
+    if nr_items == 0 then
+        return help_wrapper()
+    end
+    if items == nil then
         local msg = "Invalid command \\%s. Type \\help for help."
         return format(false, msg:format(items[1]))
     end
     return operators[items[1]](storage, unpack(items))
 end
 
+local function get_command(line)
+    -- All commands start with the escape symbol
+    if line:sub(1, 1) == '\\' then
+        return line:sub(2)
+    end
+    return nil
+end
+
 --
 -- Evaluate command on local instance
 --
@@ -256,8 +280,9 @@ local function local_eval(storage, line)
     if not line then
         return nil
     end
-    if line:sub(1, 1) == '\\' then
-        return preprocess(storage, line:sub(2))
+    local command = get_command(line)
+    if command then
+        return preprocess(storage, command)
     end
     if storage.language == 'sql' then
         return format(pcall(box.execute, line))
-- 
2.20.1

[tarantool-patches] [PATCH 3/4] box/console: Fix hang in remote console lua mode

[Cyrill Gorcunov]

If we change output mode on remote machine via
text-based session

 node a (server)
 ------
 require('console').listen('unix/:/tmp/X.sock')
 ...

 node b (client)
 ------
 require('console').connect('unix/:/tmp/X.sock')
 connected to unix/:/tmp/X.sock
 ...
 unix/:/tmp/X.sock> \set output lua

the client get stuck forever, it is because the text
wire protocol waits for yaml eos terminator which of
course never hit the peer, because lua mode uses own
one.

Thus to fix this problem we have to preprocess the text
we're passing to the server, just like we do in local
console. So we reuse command parsing code and remember
current output terminator in text_connection_mt instance.

Another problem is that named default output mode. There
could be a mixed environment where server operates in
default lua mode but client connects with yaml mode. To
break a tie we yield "\set output" command with current
output mode when establishing a connection. Since the
output format is per-session bound this doesn't affect
any other connections on a server.

Part-of #3834
---
 src/box/lua/console.lua | 50 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index 004625059..0c30ccd42 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -147,6 +147,27 @@ local function current_output()
     return d
 end
 
+local function current_eos()
+    local d = current_output()
+    return output_eos[d["fmt"]]
+end
+
+--
+-- Map output format into a "\set" command.
+local function output_cmd_string()
+    local d = current_output()
+    if d["fmt"] == "yaml" then
+        return "\\set output yaml"
+    elseif d["fmt"] == "lua" then
+        local cmd = "\\set output lua"
+        if d["opts"] == "block" then
+            cmd = cmd .. ",block"
+        end
+        return cmd
+    end
+    return ""
+end
+
 local function format(status, ...)
     local d = current_output()
     return output_handlers[d["fmt"]](status, d["opts"], ...)
@@ -330,15 +351,33 @@ local text_connection_mt = {
         -- @retval     nil Error.
         --
         read = function(self)
-            local ret = self._socket:read(YAML_TERM)
+            local ret = self._socket:read(self.eos)
             if ret and ret ~= '' then
                 return ret
             end
         end,
         --
+        -- The command might modify EOS so
+        -- we have to parse and preprocess it
+        -- first to not stuck in :read() forever.
+        preprocess_eval = function(self, text)
+            local command = get_command(text)
+            if command == nil then
+                return
+            end
+            local nr_items, items = parse_operators(command)
+            if nr_items == 3 then
+                local err, fmt, opts = output_parse(items[3])
+                if not err then
+                    self.eos = output_eos[fmt]
+                end
+            end
+        end,
+        --
         -- Write + Read.
         --
         eval = function(self, text)
+            self:preprocess_eval(text)
             text = text..'$EOF$\n'
             if not self:write(text) then
                 error(self:set_error())
@@ -393,9 +432,14 @@ local function wrap_text_socket(connection, url, print_f)
         host = url.host or 'localhost',
         port = url.service,
         print_f = print_f,
+        eos = current_eos(),
     }, text_connection_mt)
-    if not conn:write('require("console").delimiter("$EOF$")\n') or
-       not conn:read() then
+    local cmd_set_output = output_cmd_string() .. '\n'
+    local cmd_delimiter = 'require("console").delimiter("$EOF$")\n'
+    if not conn:write(cmd_set_output) or not conn:read() then
+        conn:set_error()
+    end
+    if not conn:write(cmd_delimiter) or not conn:read() then
         conn:set_error()
     end
     return conn
-- 
2.20.1

[tarantool-patches] [PATCH 4/4] box/console: Provide console.eos() api

[Cyrill Gorcunov]

Thus other modules would be able to find out what eos
marker is currently active. For example when reading
replies from remote server via text based console
protocol.

@TarantoolBot document
Title: > require('console').eos()

Returns a string with currently active end of string marker.

Part-of #3834
---
 src/box/lua/console.lua | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index 0c30ccd42..5df02dc75 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -152,6 +152,10 @@ local function current_eos()
     return output_eos[d["fmt"]]
 end
 
+local function get_current_eos()
+    return tostring(current_eos())
+end
+
 --
 -- Map output format into a "\set" command.
 local function output_cmd_string()
@@ -769,6 +773,7 @@ package.loaded['console'] = {
     delimiter = delimiter;
     set_default_output = set_default_output;
     get_default_output = get_default_output;
+    eos = get_current_eos;
     ac = ac;
     connect = connect;
     listen = listen;
-- 
2.20.1