-
Notifications
You must be signed in to change notification settings - Fork 10
Fix elasticsearch #62
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
Open
Bueller87
wants to merge
25
commits into
cadence-workflow:main
Choose a base branch
from
Bueller87:fix-elasticsearch
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+228
−9
Open
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
9d4c4cb
fix<postgres>: schema job fail open and add example (#58)
shijiesheng 502a562
docs: Convert slack links to CNCF, add link to contributing guide (#57)
vishwa-uber e63fb49
Bump Web v4.0.11 (#60)
CosminL-DEV ecc3e4e
add es examples and add to chart dependencies
shijiesheng 4d1be74
add additional comment
shijiesheng 45a7f51
add kafka values to elasticsearch example
Bueller87 9122e55
fix configmap so visibility datastore is created
Bueller87 481cf79
change cadence verion to 1.3.7
Bueller87 be5335d
Set cadence version to v1.3.6 in chart and values
Bueller87 d8bb312
fix: always create visibility database when configured
Bueller87 c85e226
run helm-docs to update Readme
Bueller87 e9f653a
chore: remove unused visibility DB config from ES example
Bueller87 7051228
fix: skip visibility DB creation when Elasticsearch is enabled
Bueller87 7f8c5c4
fix: use ES index name matching template pattern
Bueller87 9cc4bc7
fix: disable SQL visibility store for ES-only mode
Bueller87 37e3728
fix: use 'es' keyword for visibility store name in dynamic config
Bueller87 ffee53e
fix: remove advancedVisibilityStore for Kafka-based async visibility
Bueller87 297dcad
fix: explicitly set advancedVisibilityStore to empty string
Bueller87 805cd4b
fix: restore advancedVisibilityStore (required by Cadence)
Bueller87 6b2ec55
fix: provision Kafka topics at startup to avoid race conditions
Bueller87 4ad9d9d
Ran helm-docs
Bueller87 47b90b7
Merge branch 'main' into fix-elasticsearch
Bueller87 15d3d14
fix: change visibility index name
Bueller87 f7e13ac
fix: restore elasticsearch check for visibility store config
Bueller87 8a66709
fix: explicitly disable Kafka in postgres-only example
Bueller87 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,203 @@ | ||
| # Namespace note: install into namespace: cadence-postgres-es7 | ||
|
|
||
| # Allow Bitnami charts to use legacy repository images | ||
| global: | ||
| image: | ||
| tag: "v1.3.6" # Default version | ||
| security: | ||
| allowInsecureImages: true | ||
|
|
||
| # Force Cadence to use PostgreSQL for main DB | ||
| config: | ||
| persistence: | ||
| # Name of the default datastore (PostgreSQL) | ||
| defaultStore: "default" | ||
| # Disable basic SQL visibility (we're using ES-only via Kafka) | ||
| visibilityStore: "" | ||
| # Name of the advanced visibility datastore (Elasticsearch) | ||
| # Note: This defines the datastore, but dynamic config controls how it's used | ||
| advancedVisibilityStore: "es-visibility" | ||
| database: | ||
| driver: "postgres" | ||
| sql: | ||
| hosts: "cadence-release-postgresql.cadence-postgres-es7.svc.cluster.local" | ||
| port: 5432 | ||
| dbname: "cadence" | ||
| user: "cadence" | ||
| password: "changeme-strong" | ||
| tls: | ||
| enabled: false | ||
| sslMode: "" | ||
| elasticsearch: | ||
| enabled: true | ||
| version: "v7" | ||
| user: "" | ||
| password: "" | ||
| protocol: "http" | ||
| hosts: "cadence-release-elasticsearch.cadence-postgres-es7.svc.cluster.local" | ||
| port: 9200 | ||
| visibilityIndex: "cadence-visibility-es7" | ||
| tls: | ||
| enabled: false | ||
| kafka: # needed by elasticsearch integration | ||
| enabled: true | ||
| brokers: "cadence-release-kafka.cadence-postgres-es7.svc.cluster.local" | ||
|
|
||
| # Enable Cadence schema jobs | ||
| schema: | ||
| serverJob: | ||
| enabled: true | ||
| elasticSearchJob: | ||
| enabled: true | ||
|
|
||
| # Ensure Cadence uses Elasticsearch for advanced visibility | ||
| # Note: "es" is a special keyword for advanced visibility, not the datastore name | ||
| dynamicConfig: | ||
| values: | ||
| system.writeVisibilityStoreName: | ||
| - value: "es" | ||
| system.readVisibilityStoreName: | ||
| - value: "es" | ||
|
|
||
| ### subcharts values which can be omited if user has their own deployment | ||
|
|
||
| # Deploy Postgres within the same release (Bitnami subchart) | ||
| postgresql: | ||
| enabled: true | ||
| image: | ||
| registry: docker.io | ||
| repository: bitnamilegacy/postgresql | ||
| tag: "16.4.0" | ||
| pullPolicy: IfNotPresent | ||
| auth: | ||
| username: cadence | ||
| password: "changeme-strong" | ||
| database: cadence | ||
| primary: | ||
| persistence: | ||
| enabled: true | ||
| size: 8Gi | ||
|
|
||
| # Deploy Elasticsearch with single node mode | ||
| elasticsearch: | ||
| enabled: true | ||
| master: | ||
| masterOnly: false | ||
| replicaCount: 1 | ||
| resources: | ||
| requests: | ||
| cpu: 1 | ||
| memory: 1024Mi | ||
| limits: | ||
| cpu: 2 | ||
| memory: 2048Mi | ||
| data: | ||
| replicaCount: 0 | ||
| coordinating: | ||
| replicaCount: 0 | ||
| ingest: | ||
| replicaCount: 0 | ||
| sysctlImage: | ||
| enabled: false | ||
| image: | ||
| tag: 7.17.23 | ||
| repository: bitnamilegacy/elasticsearch | ||
|
|
||
| kafka: | ||
| enabled: true | ||
| image: | ||
| registry: docker.io | ||
| repository: bitnamilegacy/kafka | ||
| tag: "3.8.0" | ||
| pullPolicy: IfNotPresent | ||
| # KRaft mode configuration (Kafka without ZooKeeper) | ||
| kraft: | ||
| enabled: true | ||
| # Disable ZooKeeper since we're using KRaft | ||
| zookeeper: | ||
| enabled: false | ||
| # Controller and broker configuration for KRaft | ||
| controller: | ||
| replicaCount: 1 | ||
| persistence: | ||
| enabled: true | ||
| size: 8Gi | ||
| resources: | ||
| requests: | ||
| cpu: 500m | ||
| memory: 1Gi | ||
| limits: | ||
| cpu: 1 | ||
| memory: 2Gi | ||
| # GKE Autopilot compatibility | ||
| podSecurityContext: | ||
| fsGroup: 1001 | ||
| runAsUser: 1001 | ||
| containerSecurityContext: | ||
| runAsNonRoot: true | ||
| allowPrivilegeEscalation: false | ||
| broker: | ||
| replicaCount: 1 | ||
| persistence: | ||
| enabled: true | ||
| size: 8Gi | ||
| resources: | ||
| requests: | ||
| cpu: 500m | ||
| memory: 1Gi | ||
| limits: | ||
| cpu: 1 | ||
| memory: 2Gi | ||
| # GKE Autopilot compatibility | ||
| podSecurityContext: | ||
| fsGroup: 1001 | ||
| runAsUser: 1001 | ||
| containerSecurityContext: | ||
| runAsNonRoot: true | ||
| allowPrivilegeEscalation: false | ||
| # GKE Autopilot compatibility: disable privileged init container | ||
| sysctlImage: | ||
| enabled: false | ||
| # Kafka topic configuration for Cadence visibility | ||
| autoCreateTopicsEnable: true | ||
| numPartitions: 4 | ||
| defaultReplicationFactor: 1 | ||
| # Configure replication factors for internal topics (demo/single-node setup) | ||
| offsetsTopicReplicationFactor: 1 | ||
| transactionStateLogReplicationFactor: 1 | ||
| transactionStateLogMinIsr: 1 | ||
| # Provision topics at startup to avoid race conditions | ||
| provisioning: | ||
| enabled: true | ||
| topics: | ||
| - name: __consumer_offsets | ||
| partitions: 50 | ||
| replicationFactor: 1 | ||
| config: | ||
| cleanup.policy: compact | ||
| compression.type: producer | ||
| - name: cadence-visibility | ||
| partitions: 4 | ||
| replicationFactor: 1 | ||
| - name: cadence-visibility-dlq | ||
| partitions: 4 | ||
| replicationFactor: 1 | ||
| # Listener configuration | ||
| listeners: | ||
| client: | ||
| protocol: PLAINTEXT | ||
| controller: | ||
| protocol: PLAINTEXT | ||
| interbroker: | ||
| protocol: PLAINTEXT | ||
| # Service configuration | ||
| service: | ||
| type: ClusterIP | ||
| ports: | ||
| client: 9092 | ||
|
|
||
| # Do NOT deploy Cassandra or MySQL | ||
| cassandra: | ||
| enabled: false | ||
| mysql: | ||
| enabled: false |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
we need this default visibility
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.
When using Elasticsearch with Kafka, visibility events flow through Kafka to ES. The SQL visibility datastore is not used. (current config)
In fact I found when BOTH visibilityStore and advancedVisibilityStore are configured, Cadence enters dual-write mode where it tries to write visibility events to BOTH datastores.
I encountered multiple errors when I did this, e.g.
{"level":"warn","msg":"requested visibility mode is not available, falling back to db"}
{"level":"error","msg":"Error writing to visibility: Operation is not supported"}
Through debugging I saw this happening:
First, Cadence tried to write to BOTH SQL and ES.
But, the SQL cadence_visibility database didn't exist (schema job skipped it because ES was enabled).
Writes to the SQL visibility failed with "Operation is not supported",
and visibility was broken
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.
pretty sure there is no change needed here. I've reverted your change and the plan looks exactly the same. Please revert it.