Page MenuHomePhabricator

Allow comments longer than 255 bytes
Closed, ResolvedPublic

Assigned To
Authored By
bzimport
Jan 22 2006, 8:34 AM
Tokens
"Party Time" token, awarded by RandomDSdevel."Yellow Medal" token, awarded by Vachovec1."Yellow Medal" token, awarded by jcrespo."Like" token, awarded by Liuxinyu970226."Like" token, awarded by eugrus."Like" token, awarded by Luke081515."Like" token, awarded by Asahiko."Like" token, awarded by He7d3r.

Description

Author: avarab

Description:
To support longer edit summaries the rev_deleted field has to be changed from
TINYBLOB to BLOB and a relevant database upgrade has to be written.

This change also required bumping the minimum MySQL version to 5.0.3 (documented in the files RELEASE-NOTES-1.25 and INSTALL, brought up on wikitech-l).


Version: unspecified
Severity: enhancement
URL: http://dev.mysql.com/doc/refman/5.0/en/string-type-overview.html

Related Objects

StatusSubtypeAssignedTask
OpenNone
OpenNone
OpenNone
ResolvedNone
InvalidNone
Stalleddaniel
ResolvedAnomie
Resolvedkaldari
Resolvedjcrespo
ResolvedAnomie
ResolvedAnomie
ResolvedNone
DuplicateNone
StalledNone
OpenNone
ResolvedMarostegui
ResolvedBstorm
ResolvedMarostegui
DeclinedNone
ResolvedAnomie
ResolvedAnomie
OpenNone
ResolvedAnomie
Resolvedmatmarex

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

TL;TR options:

a) Apply the original patch, knowing it is a timed bomb
b) Separate comments on a separate table, do patches to avoid querying comments almost everwhere where possible, be aware that this is a mere quick patch, and not a proper fix, but at least it resolves the current ticket while maybe other refactoring is under work (avoid analysis-paralysis)
c) Wait for a proper refactoring of revision, which may take more time, and may even reach the same conclusion than a) for comments; but after all, if we have waited so many years, waiting some moths is not that bad

greg removed a subscriber: greg.Dec 20 2016, 8:22 PM
Arbnos added a subscriber: Arbnos.Jan 5 2017, 7:52 PM
Meno25 added a subscriber: Meno25.Jan 17 2017, 11:44 AM
mxn added a subscriber: mxn.Jan 29 2017, 7:49 PM

Was there any discussion related to this at the Developer Summit? Would this be a good topic for an ArchCom discussion?

From talking with Jdforrester, it sounds like ArchComm has already discussed this and the plan is to refactor the revision table (as part of the work on multi-content revisions) and in the process move edit summaries to another table (with a larger field). @daniel, would you say that is accurate?

Base added a subscriber: Base.Mar 6 2017, 12:14 AM
Alsee added a subscriber: Alsee.Mar 8 2017, 4:51 PM

I know next to squat about databases, so I apologize if this is incredibly dumb or obvious. Would it help to: Create the new comment table as write-only, fill it at leisure, and switch all reads&writes to the new table after it fully mirrors the existing data? Then increase the comment limit and clean up the old unused data at leisure.
That at least avoids the need to check both places during reads.

jcrespo added a comment.EditedMar 8 2017, 5:31 PM

Alsee - I do not think anybody here is worried about the deployment, but there were many discussion about design and performance.

In the end, the most important part is "show me the code"- someone has to write the code and make it work -*not* the alters/create table, the changes to all core functions (and extensions, if they touch these tables), and the fixes for all external labs-based bots and gadgets, plus maintenance scripts modifications. In most cases this was delayed because not being a simple change, other refactoring was being researched to be done at the same time- https://www.mediawiki.org/wiki/User:Brion_VIBBER/Compacting_the_revision_table_round_2 .

Your proposal is quite normal- although it is not as simple as put it- code deployment to 200 apps servers take 1-2 minutes, plus long running requests can continue running for minutes, so it is not a transactional change- filling/altering the revision table can take days -up to 5- per db server (and we have >100 of those), by the time it has been filled in, new revisions have been added, and you will never be able to catch up 100%, but again nothing that cannot be done in a slightly different way as long as the code is ready. That is why new deployed code has to work independently if a schema change is ready- it takes time to be deployed, and can be deployed asynchronously- even minutes apart- on different servers.

This is not about deployment, this is about architecture and programming. Please join the discussion about that at: T153333

Reedy removed a project: Patch-For-Review.
Reedy removed a subscriber: MediaWiki-Core-Team.
Krinkle moved this task from Untriaged to Schema changes on the Wikimedia-Rdbms board.

This will be done as stated on T174569, no longer blocked on schema change.

Ltrlg added a subscriber: Ltrlg.Dec 26 2017, 7:50 PM
Restricted Application added a subscriber: alanajjar. · View Herald TranscriptJan 27 2018, 11:09 PM
Anomie closed this task as Resolved.Mar 2 2018, 12:34 PM
Anomie claimed this task.

While there's still some backend cleanup work needed (see T166733), comments longer than 255 bytes are now allowed.

I suppose an announcement is due once T185948 is resolved?

Anomie added a comment.Mar 2 2018, 3:46 PM

T185948 pretty much is resolved, other than the vague "anything else" bullet.

Johan added a subscriber: Johan.Mar 2 2018, 4:05 PM

I'll include it in the Tech News issue that goes out on Monday.

While there's still some backend cleanup work needed (see T166733), comments longer than 255 bytes are now allowed.

This shouldn't have been just done on the fly, as this is entirely surprising for most users. Furthermore, I doubt it's even wanted like it works right now; see T6714 for example.

Alsee removed a subscriber: Alsee.Mar 3 2018, 2:01 AM

I believe that such a concern would belong in T6714 since that is the task for edit summary length.

@MGChecker I am no developer, but this task is about backend- meaning now there is a *possibility* of larger comments (or comments with multi-byte characters) to be stored on the database, something that was incredibly difficult to handle with the previous structure and required lots and lots of redesign and refactoring on a multi-month effort. A priority was set to this after it reached the top requested feature of the comunity on the yearly survey.

Now, being able to store larger comments doesn't mean you have to- the difficult part was to *allow* them, putting technical or policy restrictions is, once this backend work has been completed, in my opinion- and I want to stress I am no developer-, very very easy compared to the work to make it possible. Frontend can be accommodated, for example, to allow only 255 characters (which can take up to 1000+ bytes). Or it can be stored, but limited or filtered on display (e.g. not counting ipv6 or tags).

Of course my concern isn't the backend possibility to store longer comments, but that it's possible right now to actually enter any text up to 1000 bytes at the ui.

jcrespo added a comment.EditedMar 3 2018, 7:27 PM

Actually, for what I see, it can be quickly disabled per site by doing: https://en.wikipedia.org/wiki/Special:AbuseFilter/904 Ask an admin to apply it until a more definitive ui fix, balancing all issues (ipv6 addreses, legibility, editor preferences) are taken into account.

Please let's centralize further discussion on T6714, as this is a closed ticket, on future improvements.

Meno25 removed a subscriber: Meno25.Mar 5 2018, 10:31 AM

Change 192170 abandoned by Brian Wolff:
[WIP] Increase edit summary limit in the UI

https://gerrit.wikimedia.org/r/192170

Aklapper removed a subscriber: Anomie.Fri, Oct 16, 5:43 PM