No memory budget to create computed column

[brucemcpherson]

Describe the problem
Can't create computed column - memory budget of 0 reported.

To Reproduce

ALTER TABLE "person" 
  ADD COLUMN "searchName" STRING
AS
  (CONCAT_WS(' ',
  "firstName",
  "lastName"
  ))
STORED;

Gives this error message (there are less than 20,000 rows in the table)

concat_ws(): flow: memory budget exceeded: 10240 bytes requested, 0 currently allocated, 0 bytes in budget

and this in the log file

W190214 10:54:26.751168 1310718 sql/schema_changer.go:679  [n1,client=10.12.91.3:37928,user=root,scExec] reversing schema change 426217525867675649 due to irrecoverable error: concat_ws(): flow:
 memory budget exceeded: 10240 bytes requested, 0 currently allocated, 0 bytes in budget
I190214 10:54:26.784637 1310718 sql/event_log.go:126  [n1,client=10.12.91.3:37928,user=root,scExec] Event: "reverse_schema_change", target: 3554, info: {Error:concat_ws(): flow: memory budget ex
ceeded: 10240 bytes requested, 0 currently allocated, 0 bytes in budget MutationID:12}
W190214 10:54:27.944758 1310718 sql/exec_util.go:919  [n1,client=10.12.91.3:37928,user=root] error executing schema change: concat_ws(): flow: memory budget exceeded: 10240 bytes requested, 0 cu
rrently allocated, 0 bytes in budget

Additional data / screenshots

This is running on a Kubernetes cluster (4 VMs - 30gb memory in total)

Using your standard secure 2.1.4 yaml file. The only change is the disk size and type (the DB is currently small)

  Volumes:
   datadir:
    Type:       PersistentVolumeClaim (a reference to a PersistentVolumeClaim in the same namespace)
    ClaimName:  datadir
    ReadOnly:   false
   certs:
    Type:    EmptyDir (a temporary directory that shares a pod's lifetime)
    Medium:
Volume Claims:
  Name:          datadir
  StorageClass:  fast
  Labels:        <none>
  Annotations:   <none>
  Capacity:      32Gi
  Access Modes:  [ReadWriteOnce]
Events:          <none>

I note that the standard exec command doesn't specify a -max-disk-temp-storage. I haven't tried changing the flags at this point as I assume the default would be used. I expect the problem is here somewhere but I don't know how I should change it, however I noticed this related issue #19689 which should give this error if it was related to that flag rather than one Im seeing.

this query requires additional disk space: temp disk storage: disk budget exceeded: 5 bytes requested, 0 bytes in budget

Here's the exec command from the .yaml

      /bin/bash
      -ecx
      exec /cockroach/cockroach start --logtostderr --certs-dir /cockroach/cockroach-certs --advertise-host $(hostname -f) --http-host 0.0.0.0 --join cockroachdb-0.cockroachdb,cockroachdb-1.cock
roachdb,cockroachdb-2.cockroachdb --cache 25% --max-sql-memory 25%

Environment:

• CockroachDB version 2.1.4
• Server OS: kubernetes/gke/gcp
• Client app cockroach sql

Additional context
Have had other weird errors when using concat in sql queries in the past few days - assume this is all related

[jordanlewis]

Thanks for the report! We'll fix this right away - the issue is that the computed column backfiller doesn't have memory accounting plumbed through.

[knz]

Simple repro:

[email protected]:38003/defaultdb> create table t(x int); insert into t(x) values (1);
INSERT 1

Time: 14.061588ms

[email protected]:38003/defaultdb> alter table t add column y int as (concat_ws(x::string, ' ')) stored;
pq: concat_ws(): flow: memory budget exceeded: 10240 bytes requested, 0 currently allocated, 0 bytes in budget
[email protected]:38003/defaultdb>

[knz]

we found that any backfill work that requires a monitor is broken (eg DEFAULT). Suspecting that distsql planning ctx for background tasks is improperly initialized.

[thoszhang]

@knz that's from 2.1.4, right? I was able to reproduce on 2.1.4 and 2.1.6, but not on master.

[knz]

It did reproduce in master when I was trying this with vivek. Is it possible this was fixed since? It would be good to find out where this was fixed. Perhaps @asubiotto or @jordanlewis would know?

[asubiotto]

I think this now works probably due to removing the active mem account in #34935, I'll leave it to @jordanlewis to confirm

[knz]

OK but that being said, isn't it still a problem that the monitor root is not configured properly on distsql backfills?

[vivekmenezes]

At a minimum we should add tests for the computed column and default value use cases in a PR so we don't have a regression.

[thoszhang]

Closing this issue because #34935 has fixed this bug for builtin functions. #35665 has been filed to track the more general problem of memory monitoring for the backfiller.