Linux, mariadb docker image

Fixed version with patch on 10.2, if needed I can switch to other one. Specification only require linux env.

1ba0c5b4c49688d4ad9548daed52c9ca57cd002b is ok to push

Pushed with 396313d301b3567aea.

Broke Windows build on 5.5. Please test in own tree, before pushing into mainline.

Reverted the patch.

It doesn't compile on Windows and it doesn't compile with libedit (e.g. on p8-suse12 in buildbot).

#ifdef is a wrong fix, it makes the code to compile, but the Ctrl-C behavior is still wrong.

Example of the patch 84c9cbc36b94bcf3
Windows:

Ubuntu:

@anel won't this still terminate the client if you mash ctrl-c while a query is executing (if (interrupted_query == 2))?

Hi, I think it won't since interrupted_query == 2 according to the code means that with first interrupt the query will be killed and after that connection and this happens only for server version < 5, which doesn't recognize kill query part.

If the sigint handler is invoked three times (two times with server < 5) before com_go resets interrupted_query to 0, it will terminate the client. This can happen if you have a slow connection and press ctrl-c multiple times.

How to reproduce:

1. start a mariadb docker image
2. add a 1s delay to your docker network using tc: `tc qdisc add dev docker0 root netem delay 1000ms`
3. run `show databases;` and immediately press ctrl-c twice once works too
4. after "Ctrl-C – query killed. Continuing normally." appears, quickly press ctrl-c again.

Edit: alternatively:

1. run `show databases;` and immediately press ctrl-c twice
2. You'll get "Cltr-C – query killed. Continuing normally" twice, but you'll also get the query result and the prompt back
3. Now press cltr-c on the empty prompt and it will exit

Also, I vouch for not changing behavior on Windows. That least astonishment is Unix thing, the bash user's delight.
On Windows, either actually handle Ctrl-D (or better, Ctrl-Z which would be cultural equivalent), or leave it as-is

Accidentally terminating the client when you just wanted to cancel a long running query is bad regardless of operating system. But yes, Ctrl-Z should be handled on Windows. The mysql patch for this bug also fixed that.

I do not think anyone using Windows has complained yet.
Being the only one to jump in to comment, here is my point - I came to appreciate not having to type "quit", when I want to quit. I prefer it short . Like Ctrl-C. Any other Ctrl-whatever is in principle good to me, except Ctrl-Break - good in theory , but not existent on my keyboard

Nirbhay's patch you quoted, it does not do exactly what it promises. It does not break on Ctrl-Z on Windows. It does instead break on "Ctrl-Z-<optionally any sequence of characters>-Enter". That "Enter" is one character too much to type, and it also reads the full line, no matter how many character are there, and it does not break with Ctrl-Z the middle of the line, and all of it is the violation of principle of least astonishment, in my understanding. It is incorrect, although easy to implement this way.

The more complicated, and correct way to do that is different, and it does not involve signal handlers and what not. https://www.dpldocs.info/this-week-in-d/Blog.Posted_2019_11_25.html outlines how to implement that correctly (although it is not C, it is easy to see what is he doing).
Perhaps we should not expect Anel to make both Unix and Windows users happy in a single patch.

In fact, the Windows part, I would probably like to patch myself, but first maybe we should make more vocal and angry Linux users happy, while
not making Windows users like myself unhappy at the same time.

Lastly, not being raised in strict Unix tradition, I do not find that command line client should necessarily resemble all of the bash, and I can live with Ctrl-C ending the client in principle, and do not get all the buzz around it. Ok, one typed Ctrl-C one time too many, the client died, so what? "ARROW UP"+Enter to reconnect the client, from bash. "ARROW UP"+Enter to repeat the last command in client's history. What is all the buzz about?

Hi nkovacs, thanks for the example.
I followed but couldn't obtain the result, can you please check what is wrong ?

# Start mariadb Docker image

$ docker ps 

CONTAINER ID   IMAGE            COMMAND                  CREATED         STATUS         PORTS      NAMES

bea59acdb00c   mariadb:latest   "docker-entrypoint.s…"   2 minutes ago   Up 2 minutes   3306/tcp   mariadb-server

# Created rule for tc -> tested first to start the container after to create the rule and to remove the container on already existing rule - not working still.

# $ sudo tc qdisc add dev docker0 root netem delay 1000ms

# Show the rule

 $ tc qdisc show|grep docker0

qdisc netem 8001: dev docker0 root refcnt 2 limit 1000 delay 1.0s

# 1) Try from within container -> couldn't get latency and preform quick ^C

$ docker exec -it mariadb-server bash

MariaDB [(none)]> show databases;

+--------------------+

| Database           |

+--------------------+

| information_schema |

| mysql              |

| performance_schema |

| sys                |

+--------------------+

4 rows in set (0.000 sec)

# 2) Try with exec -> again couldn't get latency and preform quick ^C

$ docker exec -it mariadb-server mariadb -uroot -psecret

MariaDB [(none)]> show databases;

+--------------------+

| Database           |

+--------------------+

| information_schema |

| mysql              |

| performance_schema |

| sys                |

+--------------------+

4 rows in set (0.000 sec)

wlad thanks for the reference without sighandler.
The patch is for ^C only because it failed last time for Windows (I think it works now for Windows), of course we can remove Win patch, but also I can take a look for ^Z - without sighandler, if we decide to go with that and someone confirms or to do that in other MDEV, what may be atm better solution .

I ran the mariadb client from outside the docker container. I think that may be the issue.

Like this ?

# 2) Try with exec -> again couldn't get latency and preform quick ^C

$ docker exec -it mariadb-server mariadb -uroot -psecret

No. I started the mariadb image with -p=3306:3306, then ran the client on my machine, not docker, with -hlocalhost -P3306 --protocol=tcp

Thanks, but still the same:

$ tc qdisc show|grep docker

qdisc noqueue 0: dev docker0 root refcnt 2 

$ docker container run --rm -p=3306:3306 --name mariadb-server -e MYSQL_ROOT_PASSWORD=secret -d mariadb:latest

$ docker ps

CONTAINER ID   IMAGE            COMMAND                  CREATED         STATUS         PORTS                                       NAMES

2dc4fa93c179   mariadb:latest   "docker-entrypoint.s…"   4 minutes ago   Up 4 minutes   0.0.0.0:3306->3306/tcp, :::3306->3306/tcp   mariadb-server

$ ./client/mysql -uroot -psecret -hlocalhost -P3306 --protocol=tcp

MariaDB [(none)]> show databases;

+--------------------+

| Database           |

+--------------------+

| information_schema |

| mysql              |

| performance_schema |

| sys                |

+--------------------+

4 rows in set (0.000 sec)

$ ps aux|grep mysql

999        2911  0.0  0.4 1408680 76784 ?       Ssl  10:51   0:00 mysqld

Update:

Obtained after setting again $ sudo tc qdisc add dev docker0 root netem delay 1000ms.
Thanks, will take a look.

Current behavior (without patch) is ok IMHO - no matter how many times you press ^C you will get that query is killed and without result. When you press single ^C you exit, what is expected.

anel, that Ctrl-C or Ctrl-D, or Ctrl-Z does not matter that much. I just do not want to type "quit", and that's it . I need some Ctrl-Whatever (-no, not terminated by Enter) to quit fast. From your pictures, it does not seem that there is an option to do that, so we might as well ignore bashism and Linuxism on Windows, and retain the previous behavior

anel, I wouldn't say that exiting on ^C is expected. In fact I totally agree with the original description:

Control-c should not exit the client. It should cancel partial input or abort the current statement if there is one, but it should not do anything else otherwise.
Control-d should exit the client if the input line is empty.

wlad While I understand your request for having an easy way to "quit fast" on windows, I do not see any evidence in Windows land where Ctrl-C should do that. Windows PowerShell, as an interactive shell does not quit when issued a Ctrl-C.

The only other signal that Windows handles as part of it's Console signal handlers is Ctrl-Break. MySQL coded Ctrl-Break to behave just like Ctrl-C (unix style). I've made a patch that causes the client to exit if you hit Ctrl-Break.

Is that acceptable for you? I really would like to keep ourselves compatible between MySQL and our implementation with Ctrl-C.

A patch that implements this, with a commit comment explaining exact behaviour can be found here:
https://github.com/MariaDB/server/commit/8b16448e9417d44132dbac6622af83f64f25486d
serg Can you please also double check the commit comment, to make sure you agree with the behaviour proposed? I would like to push this directly into 10.4, as a bug fix, not a "new feature". Given our last meetup in Helsinki, I think that this should be in line with community expectations, but I would like your sanity check.

cvicentiu . CTRL-C ends a console program , by default, if you do not program any handlers for it. Is not this an evidence? Secondly, I would like to see a pointer to a public discussion where a Windowshead explains his opinions on how CTRL-C in "interactive programs" should work. I did not see it in the last 30 years, and I'd say there is no religious views in this community about it, unlike Linux-community, that forces its views on anything else. I , personally, found current behavior convenient, and am disturbed if someone is changing it,

I encourage you to find the authoritative guidelines on command line application on Windows. Authoritative is from someone who actually uses Windows daily (I'm afraid there is not many option in MySQL and MariaDB land )

if there are no guidelines, we can see what other standard Windows console tools do.
cmd.exe, powershell, ftp, netsh — none of them exists on either Ctrl-C or Ctrl-Break.
choice.exe exits on both, it doesn't distinguish between them either.

If we must have a Ctrl-something to quit mariadbd, I'd suggest Ctrl-D then. It's totally non-Windows, no Windows tool does that, so likely no Windows user will press is and expect a specific outcome. And it's what we on Linux do, so, familiar.

There are no guidelines, but there are existing users, who will find this change "disruptive and unintiutive" on Windows.. I'm a bona-finde user who uses this OS everyday, and our tools there, too. I also happen not to have a Break on my keyboard, and it seems that Break is now well hidden on keyboards. Ctrl-D does not work. Ctrl-Z does not work. I would rather not not write "quit". So, of all Windows users we have feedback so far, none found the old behavior disruptive, and the all vocal ones found this proposed change disruptive.

So here is the deal - can we do exactly nothing about it Windows in 10.4, or whatever old version it was predestined to be fixed. It would be impolite to break tools in the very last version before release is going EOL. In 10.11+ I'll file a followup, to break mysql on Ctrl-D or Ctrl-Z or both. Because this version already uses ReadConsoleW, making it possible to catch Ctrl-<something> for <something> that is not "C" (via last parameter, CONSOLE_READCONSOLE_CONTROL , which only works in Unicode "W" version)

• agree about 10.4
• I couldn't understand whether you agree with the suggestion 'Ctrl-C and Ctrl-Break do not exit the tool, Ctrl-D does", could you state that explicitly, please?

Yes, Ctrl-D interrupts, Iood enough to me, although python uses Ctrl-Z on Windows some reason. I have no idea about Ctrl-Break, I can't generate it, so I do not care either way.

Checked statement: "cmd.exe, powershell, ftp, netsh — none of them exists on either Ctrl-C or Ctrl-Break." netsh does actually

Implemented this by only changing Linux behaviour for CTRL-C and adding some extra windows support for non-interactive signals sent by the OS during terminal close, log off or shut down, like in MySQL's client implementation.