Skip to content

Conversation

bobbyiliev
Copy link
Collaborator

No description provided.

@bobbyiliev bobbyiliev requested a review from jubrad April 14, 2025 18:14
Copy link
Contributor

@jubrad jubrad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good. G2G if you validated this one works through one of the other tf modules.

@bobbyiliev
Copy link
Collaborator Author

Change looks good. G2G if you validated this one works through one of the other tf modules.

I've just tested this with the AWS module, the env var is set:

kubectl describe pod  -n materialize-environment mzz9zuhf0269-environmentd-1-0
   ...
    Environment:
      MZ_METADATA_BACKEND_URL:      ...
      MZ_PERSIST_BLOB_URL:        ...
      AWS_REGION:                   us-east-1
      ENABLE_COLUMNATION_LGALLOC:   true
      AWS_STS_REGIONAL_ENDPOINTS:   regional
      AWS_ROLE_ARN:                 arn:aws:iam::....:role/bobby-test...
      AWS_WEB_IDENTITY_TOKEN_FILE:  /var/run/secrets/eks.amazonaws.com/serviceaccount/token

However:

mz_system=> SHOW enable_columnation_lgalloc;
 enable_columnation_lgalloc 
----------------------------
 off

@bobbyiliev
Copy link
Collaborator Author

Change looks good. G2G if you validated this one works through one of the other tf modules.

Just updated this PR to have the following format: MZ_SYSTEM_PARAMETER_DEFAULT=key=value;.. however this still does not get picked up correctly.

The env is passed along to the envd pod:

materialize@mz9hv0020kkf-environmentd-1-0:/$ printenv | grep MZ_ 
MZ_SYSTEM_PARAMETER_DEFAULT=enable_columnation_lgalloc=true

But the value is still not updated:

mz_system=> show enable_columnation_lgalloc;
 enable_columnation_lgalloc 
----------------------------
 off

Any suggestions?

@jubrad
Copy link
Contributor

jubrad commented May 12, 2025

This is due to a conflict with other default values leveraging --system-parameter-default
We should

  1. proceed with this PR (it's good as is)
  2. also include environmentdExtraArg which can be specified like so
environmentdExtraArgs:
  - --system-parameter-default=allow_real_time_recency=true
  - --system-parameter-default= enable_connection_validation_syntax=true

This seems to work to:

  1. provide env vars to environmentd (these won't subsequently get passed to clusterd which is a bit of a bummer)
  2. allow us to set default system params directly from terraform

@bobbyiliev bobbyiliev force-pushed the add-extra-envd-args-var branch from fee0943 to 7014e7e Compare May 13, 2025 05:34
@bobbyiliev bobbyiliev merged commit 7a9b3b9 into main May 13, 2025
1 check passed
@bobbyiliev bobbyiliev deleted the add-extra-envd-args-var branch May 13, 2025 05:40
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