Creating parameters with large text values causes postgres error -> exceeds maximum 2712 for index "unique_param"

mkrug1981 commented 5 years ago

I'm submitting a ...

[x] bug report
[ ] new feature / enhancement request
[ ] improvement request (usability, performance, tech debt, etc.)
[ ] other

Traffic Control components affected ...

[ ] CDN in a Box
[ ] Documentation
[ ] Grove
[ ] Traffic Control Client
[ ] Traffic Monitor
[x] Traffic Ops
[ ] Traffic Ops ORT
[ ] Traffic Portal
[ ] Traffic Router
[ ] Traffic Stats
[ ] Traffic Vault
[ ] unknown

Current behavior:

Creating a parameter with a large text value causes following postgres database error.

ERROR:  54000: index row size 2728 exceeds maximum 2712 for index "unique_param"
HINT:  Values larger than 1/3 of a buffer page cannot be indexed.
        Consider a function index of an MD5 hash of the value, or use full text indexing.
LOCATION:  _bt_findinsertloc, nbtinsert.c:583
STATEMENT:  UPDATE
        parameter SET
        config_file=$1,
        id=$2,
        name=$3,
        value=$4,
        secure=$5
        WHERE id=$6 RETURNING last_updated

Expected / new behavior:

Possibility to store large text values, like complex lua code files as parameter without errors. Not sure but disabling indexing for the text field might help?

Minimal reproduction of the problem with instructions:

Create a parameter with a text value with more as 2800 characters will trigger the issue.

$ cat /dev/random | LC_CTYPE=C tr -dc "[:alpha:]" | head -c 2680
$ curl -vv -o /dev/null https://tp.random.com/api/1.4/parameters/3996 --cookie 'mojolicious=xxxxxxxxxxxxx' -H 'application/json' -d '{"configFile":"pdl_config.lua","id":3996,"name":"data","secure":false,"value":"CgfLkNHOjxGmFrXLxTDKHmEdGrkiMgyQPEQQuYRKuvCLtzuWKiiLveOLYdQZJMApYELhxIMhvkbBszzJeswNCOzedZiVAQlLQSYzLoCfJzVOUppseXePDjoteIGcDleiKZRyjxcirYMSNqwHfyDIElpjOGyZkvKoXVsDqyJAsQxYtMbJAiUcbOLibVWYRbflDjzNwPVinmkrHmWbwTfFIqKHjzFjmarNAkUMzwQZyoeqWikAAVvmroAzrKnODlWjIoewsgiYYiilUMVTcXXNLHxpBBgRsitHVPnnZUcdxMCDkykoTiTWdSHUbLRgUbFivjaorGRpxQtYvyjgkvUwShakpzyoTIolHeJmaBbHJBkGvczGsSRDPpuYtPSWECotFTOJShUAlailYJlndWcEvjqwxohRPMGNvsRZGhiVhGxvtghsBsXxdgsztQsFWVHzdFmSiystfXEjYILFYpmdjvBLtYWZkaRqhUwvYXwmmiEtmXESskxcAmuVPCPVlXgFfKixmAHcedksNaKiEIHXzJhUviZkFPewDcVwlQQhizEbwcEtJlMKNmHXxuynhRsgIcMuNUzwWMQjmFpLecGIWxCeclWBOnOpYjKsgyCxnfFzEVofpnLEWayxwlNrBXOcoXszlwahFzQfcIqpYmRGtMVBtAfRKcMjeokTXyOzeuqLibkWxdIkqOyYZDXMtQctocZDVMSCsvdHrrFLgjaIyvpEbeOMTFWqeKdkqzRIyVMwscUkqUkvsRQwYHuhcnMdcgzMgmbxNUOKdwnPFJskKmgryJnnLQIHMvElTCetupukcVSznkstrDcelRSTpiauKuVcSNSWLJLWSosaFCgsJSJqyrVOXKGOpZuKmEYzuzkiOrBiOjdHnRjsWDUfbvKYaXlHSRguaMwapLFGkTgCMGjWIXQTzuMUkbXKtlFEsDAJNOqQyaLduAbHzkLjeaqnpSKhnjydjGquDCFaYIykopTjTlaCnRyPmSgKJxIlqaKzbsLUslrNOSFcJdQXXajiAXBlsUKPWHIKMUEPlkUptfFfKRYPXGoSptvTrFQSDsMmqfevBHGqxJIThllVCoedfLkoftAcQGTMFBqqYJldfbjgkpeKEVyfXvVGVoWprGaCczUaBwDoQyUEKAFsoJakkeSJtuBdpbcXWFLOdLlYMXdAuEVmIjXFtJwnZFYaEKeFEFnswzoyywUPXOksDCgxmSeHBXskgixuVLQLIBeNFPDuzSwqTVXCMcdjzqcFCiKMcQuYlTfEWvZirgzVivIvYnchsOqOOQgnObDFCNmYblGTaMUJFeaHDRdwegZAMUXaNOCQGVyvPgjRVlRKravJxegEwAbsCyCTSKjHzqWgGLKswddRLMpFGrtKnWFwuwGUURNJXctszhEWDBmSNqHTkrvAwgssGtwSeRcRAlxVSJpCHVMpYrwgYLrtUSnbekPloEqGibVfKNhGNfRsniDbBRLrcGBkVwAqIrISIfbqQJBKIJFAGtXxxbcLsvgTvkvaSXZGoYHhIaZuiqISmLZgEbJiwHZblBhBXGjvypThZpCPHajotxAaKzhoXDaXclhyjlEPrjQTVPRGRxRVduvVfRVnMbBQYRmlDPQrcMHGPIOBRUuPEiBazastFLAWjAIWADsQGxlRxBfcWoScVXnUVTIXmXelqEWRyJdpRgIonkvcWudtsoNciFKbTHXtamwvavciASTRqdcKNinsAkyhXDizRrWzVtUHHoDXBSMKPkrjnLBTqgLNKeVKHRTjSnHHkbqcQGQfUhwHYbbgrHyNJfUWYPaDeITLMHKBPfxxAjDxiFScIhyVnQngIpvHsWyouUMXUWMOfMMDzrGioGleaEudkgcypjgvyLcsrDjoHWiJZbZbVMIYFcieRudOAMqyffOswOOxkcgGLALonwPOaaaDmcyruFnxVYhQzbcKmAxNqlkdZATgQKCotOGRqMqMulsKxDiivkdpBtmiSNHtnapdZyaOdAxqoUbYrcNySKuxmYMYKeGRotDqRwlaMxiWAZieMcLyfLrlNhMYtgOKcyPSPSPoFlBdwJYvLfjerSWBiiKneWDkfifJhcXXkIcDyNsojTSxDqnkaCGxwakfzyPSbEQroheMwRmEGevxFcoUvBhuBeBrcvOjSgWbMsxnLnsGzGRHlNFKaFmGLsKSCetsnEHAFYTtSfAJDUAwgUZaQnAfoqcVyolrFCwKqfQmNurwdqUJeSfbVdadGouoVYRmcWSacnJCYUmGIrfRJdohyKQlpTMqyxZGikKljRsVJFqPRwFcqZqXrdAgRemvNhzctkckDrPphxLXCBVluVrVcefmgviFLLmmolRCSBSRlMFxQrAhUQhneiyvwoTfWnsgXsfkugQpxDcqoUrSBFvXAfceULBhJnznbBesIUcpKOEVXIOiLQJQyLTKSkNcxFVDvpbKNKNTwwxmxOVxmyLqiTPHDcpxYKtuObSbmWmDNWaJtpoXGyfyJCWGuQawEyhrxOGFmbrRvkJLoIHcaniDbqYkgacKTsKamHpuxZdzDJaRYCddhpQUdHAgIUQSslyGattYgzqlrxqQwLsRpPBIkTJfINVUFHKykbTshfkrBiNZvTnzXHIYTpgyOSPPLLSIIMylqcjfzVxLuzzTQZNgloojQgOBHeTmFYHkNMoCqfghmxBLLaIQLB"}' -k -X PUT 2>&1 | egrep 'HTTP/'

> PUT /api/1.4/parameters/3996 HTTP/1.1
< HTTP/1.1 100 Continue
< HTTP/1.1 500 Internal Server Error

Anything else:

Some database details output for the tables and parameter settings

$ psql -h x.x.x.x -U traffic_ops
Password for user traffic_ops: 
psql (9.6.14, server 9.6.13)
SSL connection (protocol: TLSv1.2, cipher: ECDHE-RSA-AES256-GCM-SHA384, bits: 256, compression: off)
Type "help" for help.

traffic_ops=# 

traffic_ops=# \l
                                    List of databases
    Name     |    Owner    | Encoding |   Collate   |    Ctype    |   Access privileges   
-------------+-------------+----------+-------------+-------------+-----------------------
 grafanadb   | grafana     | UTF8     | en_US.UTF-8 | en_US.UTF-8 | 
 postgres    | postgres    | UTF8     | en_US.UTF-8 | en_US.UTF-8 | 
 template0   | postgres    | UTF8     | en_US.UTF-8 | en_US.UTF-8 | =c/postgres          +
             |             |          |             |             | postgres=CTc/postgres
 template1   | postgres    | UTF8     | en_US.UTF-8 | en_US.UTF-8 | =c/postgres          +
             |             |          |             |             | postgres=CTc/postgres
 traffic_ops | traffic_ops | UTF8     | en_US.UTF-8 | en_US.UTF-8 | 
(5 rows)

traffic_ops=# 

postgres=# \c traffic_ops
psql (9.6.14, server 9.6.13)
SSL connection (protocol: TLSv1.2, cipher: ECDHE-RSA-AES256-GCM-SHA384, bits: 256, compression: off)
You are now connected to database "traffic_ops" as user "traffic_ops".
traffic_ops=# 

traffic_ops=# \d+ parameter
                                                         Table "public.parameter"
    Column    |           Type           |                       Modifiers                        | Storage  | Stats target | Description 
--------------+--------------------------+--------------------------------------------------------+----------+--------------+-------------
 id           | bigint                   | not null default nextval('parameter_id_seq'::regclass) | plain    |              | 
 name         | text                     | not null                                               | extended |              | 
 config_file  | text                     |                                                        | extended |              | 
 value        | text                     | not null                                               | extended |              | 
 last_updated | timestamp with time zone | not null default now()                                 | plain    |              | 
 secure       | boolean                  | not null default false                                 | plain    |              | 
Indexes:
    "idx_89644_primary" PRIMARY KEY, btree (id)
    "unique_param" UNIQUE CONSTRAINT, btree (name, config_file, value)
    "idx_89644_parameter_name_value_idx" btree (name, value)
Referenced by:
    TABLE "profile_parameter" CONSTRAINT "fk_atsprofile_atsparameters_atsparameters1" FOREIGN KEY (parameter) REFERENCES parameter(id) ON DELETE CASCADE
    TABLE "cachegroup_parameter" CONSTRAINT "fk_parameter" FOREIGN KEY (parameter) REFERENCES parameter(id) ON DELETE CASCADE
Triggers:
    on_update_current_timestamp BEFORE UPDATE ON parameter FOR EACH ROW EXECUTE PROCEDURE on_update_current_timestamp_last_updated()

traffic_ops=#

ocket8888 commented 5 years ago

Indexing is - I think - how we maintain uniqueness. That could be changed to a constraint, but I think the other hints it gives about indexing a hash or "use[ing] full-text indexing" are worth looking into.

mitchell852 commented 3 years ago

if the intent is to limit parameter values to 2712 characters, better validation is needed in TO.

zrhoffman commented 3 weeks ago

The constraint was added in https://github.com/apache/trafficcontrol/commit/994f6f6594f2. Maybe it was intended as a solution to needing 2 parameters with the same name but different content?

If so, the constraint does not seem helpful and should be dropped entirely.

apache / trafficcontrol