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.

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

eliminate unnecessary SHOW TABLES which breaks on Vitess #1182

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

nharkins
Copy link
Contributor

Description

mycli currently makes two queries to the database to prep its auto-completion feature after connecting:

  1. SHOW TABLES
  2. select TABLE_NAME, COLUMN_NAME from information_schema.columns ...
    The first is actually unnecessary, as the second also contains the table names.
    Arguably the above is trivial and not worth changing, HOWEVER the above results in an error on Vitess:
$ mycli --host 10.183.249.211 --port 15306 -Dloadtest
MySQL 8.0.31
mycli 1.24.3
Home: http://mycli.net/
Bug tracker: https://github.com/dbcli/mycli/issues
Thanks to the contributor - Shoma Suzuki
MySQL [email protected]:loadtest> Exception in thread completion_refresh:
Traceback (most recent call last):
  File "/usr/lib/python3.10/threading.py", line 1016, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.10/threading.py", line 953, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/lib/python3/dist-packages/mycli/completion_refresher.py", line 63, in _bg_refresh
    refresher(completer, executor)
  File "/usr/lib/python3/dist-packages/mycli/completion_refresher.py", line 102, in refresh_tables
    completer.extend_columns(executor.table_columns(), kind='tables')
  File "/usr/lib/python3/dist-packages/mycli/sqlcompleter.py", line 172, in extend_columns
    metadata[self.dbname][relname].append(column)
KeyError: '_vt_hold_cb11180fb29d11efbe790affca8db0b9_20241206001421'
MySQL [email protected]:loadtest>

Vitess is a distributed database layer for sharding MySQL.
Their vtgate component behaves as a drop-in replacement for MySQL.
When a query is issued, it determines to which shard(s) it needs to proxy the query.
In the case of SHOW TABLE and information_schema.*, it picks a shard at random on every query.

Ideally, the schema should be identical across all Vitess shards, and certainly is for any user-created data on Vitess.
However, the maintainers have introduced an "Online DDL" feature which creates those _vt_hold_... tables for its own internal functions, which have a timestamp suffix that is slightly different between shards, hence the error above.
Arguably, what I have described is a Vitess bug, and I am also following up with them to see what can be done.

However, in light of the above SHOW TABLES call being totally unnecessary, I think it increases the usability of mycli to just make the change so it doesn't error on Vitess, because it will then only issue one query, which will go to one shard, which will be consistent for the normal tables, and only error if a user attempts to autocomplete the Vitess internal tables, which one should not expect to work anyway, as they have different table names between shards.

Note that the change I've done here is minimal wrt lines modified, yet it effectively undoes any performance benefit of using a generator. The generator feels a little unnecessary to me personally, but perhaps there once was someone with a mysql database with millions of columns. If you agree about eliminating SHOW TABLES, then we can iterate on this and make a different PR with a larger number of lines changed but preserving the generator.

Checklist

let's discuss the change first, and if you agree on the implementation, i'll fill these out.

  • I've added this contribution to the changelog.md.
  • I've added my name to the AUTHORS file (or it's already there).

@amjith amjith merged commit 8099add into dbcli:main Dec 11, 2024
5 checks passed
@amjith
Copy link
Member

amjith commented Dec 11, 2024

Thank you, Neil.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants