Skip to content
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

IWF-232: Add disabling sticky cache option #473

Merged
merged 12 commits into from
Nov 4, 2024
26 changes: 26 additions & 0 deletions .github/workflows/ci-cadence-integ-test-disable-sticky.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
name: Cadence Integration Test
on:
pull_request:
push:
branches:
- 'main'

jobs:
tests:
name: "Integration testing with sticky cache disabled"
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
test-subset:
- "a-m"
- "n-z"
steps:
- uses: actions/checkout@v3
- name: "Set up cadence environment"
run: docker compose -f docker-compose/ci-cadence-dependencies.yml up -d
- name: "Test against cadence"
run: make ci-cadence-integ-test-disable-sticky startsWith=${{ matrix['test-subset'] }}
- name: Dump docker logs
if: always()
uses: jwalton/gh-docker-logs@v2
31 changes: 31 additions & 0 deletions .github/workflows/ci-temporal-integ-test-disable-sticky.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
name: Temporal Integration Test
on:
pull_request:
push:
branches:
- 'main'

jobs:
tests:
name: "Integration testing with sticky cache disabled"
runs-on: ubuntu-latest
permissions:
# Give the default GITHUB_TOKEN write permission to commit and push the
# added or changed files to the repository.
contents: write
strategy:
fail-fast: false
matrix:
test-subset:
- "a-m"
- "n-z"

steps:
- uses: actions/checkout@v3
- name: "Set up temporal environment"
run: docker compose -f docker-compose/ci-temporal-dependencies.yml up -d
- name: "Test against temporal"
run: make ci-temporal-integ-test-disable-sticky startsWith=${{ matrix['test-subset'] }}
- name: Dump docker logs
if: always()
uses: jwalton/gh-docker-logs@v2
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,14 @@ cadenceIntegTests:
ci-cadence-integ-test:
$Q go test -v ./integ -search=false -temporal=false -dependencyWaitSeconds=180

ci-cadence-integ-test-disable-sticky:
lwolczynski marked this conversation as resolved.
Show resolved Hide resolved
$Q go test -v ./integ -run "(?i)^Test[${startsWith}]" -search=false -temporal=false -dependencyWaitSeconds=180 -disableStickyCache

ci-temporal-integ-test:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you test does -disableStickyCache=true work? I thought it has to be -disableStickyCache because it's a bool flag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does work, but I can simplify it to disableStickyCache

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

$Q go test -v ./integ -cover -coverprofile coverage.out -coverpkg ./service/... -search=false -cadence=false -dependencyWaitSeconds=60
$Q go test -v ./integ -cover -coverprofile coverage.out -coverpkg ./service/... -search=false -cadence=false -dependencyWaitSeconds=60

ci-temporal-integ-test-disable-sticky:
$Q go test -v ./integ -run "(?i)^Test[${startsWith}]" -cover -coverprofile coverage.out -coverpkg ./service/... -search=false -cadence=false -dependencyWaitSeconds=60 -disableStickyCache

ci-all-tests:
# Fails CI when used with -coverprofile flag due to tests that panic; see https://go.dev/doc/build-cover#panicprof
Expand Down
2 changes: 2 additions & 0 deletions integ/flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,5 @@ var searchWaitTimeIntegTest = flag.Int("searchWaitMs", 2000, "the amount of time
var temporalHostPort = flag.String("temporalHostPort", "", "temporal host port")

var dependencyWaitSeconds = flag.Int("dependencyWaitSeconds", 60, "the number of seconds waiting for dependencies to be up(Cadence/Temporal)")

var disableStickyCache = flag.Bool("disableStickyCache", false, "disable Temporal/Cadence sticky execution")
12 changes: 10 additions & 2 deletions integ/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,11 @@ func doStartIwfServiceWithClient(config IwfServiceTestConfig) (uclient uclient.U

// start iwf interpreter worker
interpreter := temporal.NewInterpreterWorker(createTestConfig(config), temporalClient, service.TaskQueue, config.MemoEncryption, dataConverter, uclient)
interpreter.Start()
if *disableStickyCache {
interpreter.StartWithStickyCacheDisabledForTest()
} else {
interpreter.Start()
}
return uclient, func() {
iwfServer.Close()
interpreter.Close()
Expand Down Expand Up @@ -155,7 +159,11 @@ func doStartIwfServiceWithClient(config IwfServiceTestConfig) (uclient uclient.U

// start iwf interpreter worker
interpreter := cadence.NewInterpreterWorker(createTestConfig(config), serviceClient, iwf.DefaultCadenceDomain, service.TaskQueue, closeFunc, uclient)
interpreter.Start()
if *disableStickyCache {
interpreter.StartWithStickyCacheDisabledForTest()
} else {
interpreter.Start()
}
return uclient, func() {
iwfServer.Close()
interpreter.Close()
Expand Down
15 changes: 15 additions & 0 deletions service/interpreter/cadence/worker.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cadence

import (
"fmt"
"github.com/indeedeng/iwf/config"
"log"

Expand Down Expand Up @@ -37,7 +38,15 @@ func (iw *InterpreterWorker) Close() {
iw.worker.Stop()
}

func (iw *InterpreterWorker) StartWithStickyCacheDisabledForTest() {
iw.start(true)
}

func (iw *InterpreterWorker) Start() {
iw.start(false)
}

func (iw *InterpreterWorker) start(disableStickyCache bool) {
config := env.GetSharedConfig()
var options worker.Options

Expand All @@ -55,6 +64,12 @@ func (iw *InterpreterWorker) Start() {
options.MaxConcurrentDecisionTaskPollers = 10
}

// When DisableStickyCache is true it can harm performance; should not be used in production environment
if disableStickyCache {
options.DisableStickyExecution = true
lwolczynski marked this conversation as resolved.
Show resolved Hide resolved
fmt.Println("Cadence worker: Sticky cache disabled")
}

iw.worker = worker.New(iw.service, iw.domain, iw.tasklist, options)
worker.EnableVerboseLogging(config.Interpreter.VerboseDebug)

Expand Down
15 changes: 15 additions & 0 deletions service/interpreter/temporal/worker.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package temporal

import (
"fmt"
"github.com/indeedeng/iwf/config"
"log"

Expand Down Expand Up @@ -35,7 +36,15 @@ func (iw *InterpreterWorker) Close() {
iw.worker.Stop()
}

func (iw *InterpreterWorker) StartWithStickyCacheDisabledForTest() {
iw.start(true)
}

func (iw *InterpreterWorker) Start() {
iw.start(false)
}

func (iw *InterpreterWorker) start(disableStickyCache bool) {
config := env.GetSharedConfig()
var options worker.Options

Expand All @@ -55,6 +64,12 @@ func (iw *InterpreterWorker) Start() {
options.MaxConcurrentWorkflowTaskPollers = 10
}

// When DisableStickyCache is true it can harm performance; should not be used in production environment
if disableStickyCache {
worker.SetStickyWorkflowCacheSize(0)
lwolczynski marked this conversation as resolved.
Show resolved Hide resolved
fmt.Println("Temporal worker: Sticky cache disabled")
}

iw.worker = worker.New(iw.temporalClient, iw.taskQueue, options)
worker.EnableVerboseLogging(config.Interpreter.VerboseDebug)

Expand Down
Loading