-
Notifications
You must be signed in to change notification settings - Fork 45
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
Use text instead of string as it can be limiting for paths length #1215
Use text instead of string as it can be limiting for paths length #1215
Conversation
… for paths lenght""
Note: I tried to add this on trello but it did not format correctly. Below I show some steps in order to see the problem related to the DB. Altered the table by optimizing:https://medium.com/@ashik.ck/how-to-re-index-mysql-database-after-restored-356788eeba62 No change Used some variations of string (varchar)1st mirror around 1m
When using text1st mirror up to 6 minutes
These are set on the rails migration file as :string, limit: [limit] This is a InnoDB limitation: commands used: Around 9GB Total repositories -> 3358 |
Having paths longer than 255 chars is really an exception, that's why it was only reported now. I'd suggest to set the column to varchar(512), and test for the performance impact. If it's low impact (expected), then I'd change it to 512. |
e9b74e1
to
1af565e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Do we have a timing result for varchar(512) to compare? |
It is basically the same time. Even 1 second less on average (from 13 to 12) I tested this on a different network and in a docker container. |
No description provided.