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

Flake in TestTxPool_ExpiredTxs_Timestamp #1207

Closed
1 task
evan-forbes opened this issue Feb 1, 2024 · 0 comments · Fixed by #1573
Closed
1 task

Flake in TestTxPool_ExpiredTxs_Timestamp #1207

evan-forbes opened this issue Feb 1, 2024 · 0 comments · Fixed by #1573
Labels
C:CI Continuous integration and automation related issues cat 🐈 flake A test flake that occurred on Github CI WS: Maintenance 🔧

Comments

@evan-forbes
Copy link
Member

evan-forbes commented Feb 1, 2024

Description

We're currently experiencing a flake see https://github.com/celestiaorg/celestia-core/actions/runs/7742611329/job/21112148913?pr=1206

--- FAIL: TestTxPool_ExpiredTxs_Timestamp (0.01s)
    pool_test.go:581: Transaction 54784B65797B394244464131313144463739394636464239434641344439363445313846433330463434433434303534394537323146353131453138343334443337353342347D should still be in the mempool, but is not
    pool_test.go:581: Transaction 54784B65797B393631414535343545453445454633303133334239414141373743344144363236444644343537343338303936453043413744453433383843343032303433437D should still be in the mempool, but is not
    pool_test.go:581: Transaction 54784B65797B423632424531393838464339384644334145463835383639364441353535373033384634444644333934444338303632394643363139433639383334304136377D should still be in the mempool, but is not
    pool_test.go:581: Transaction 54784B65797B353438324541443442424139433638324533433642354134393737423646464233343331383633434642413232453639363432463530343333344636374346447D should still be in the mempool, but is not
    pool_test.go:581: Transaction 54784B65797B334244373936344230374237463542383046463830434339303341423337373538344238363838434531313033313845304241434642363438344333453742437D should still be in the mempool, but is not
    pool_test.go:581: Transaction 54784B65797B453834344233343638464638363932314532463736454231364635423338313538434233394438313136434245453133464531384246353332333141303841427D should still be in the mempool, but is not
FAIL
coverage: 86.6% of statements
FAIL	github.com/cometbft/cometbft/mempool/cat	18.396s

Acceptance Criteria

  • Investigate and resolve the flake
@evan-forbes evan-forbes added C:CI Continuous integration and automation related issues flake A test flake that occurred on Github CI labels Feb 1, 2024
rootulp pushed a commit that referenced this issue Jan 2, 2025
## Description

Closes #1207  

## Rationale
After a lot of trial and errors and some research, the rationale behind
this change of flaky test is:

Because the TTL is 5ms which is very short, we need to have a more
precise pruning interval to ensure that the transactions are expired, so
that the expired event is caught quickly enough, while the second batch
of transactions are not expired to prevent flaky behaviors.


## Testing method 
```go test -run TestTxPool_ExpiredTxs_Timestamp github.com/tendermint/tendermint/mempool/cat -mod=readonly -race -count 100```

I run this command 200 times and see no failing one, which is kind of reliable compare to failing rate of 15-20% of old implemetation 


---

#### PR checklist

- [x] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
  [unclog](https://github.com/informalsystems/unclog) to manage our changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:CI Continuous integration and automation related issues cat 🐈 flake A test flake that occurred on Github CI WS: Maintenance 🔧
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants