Remove cadence/common/tokenbucket

[natemort]

This appears to only be used in the CLI. It seems likely that we could replace it with a normal RateLimiter and remove this custom implementation. Anything used in the CLI is certainly not performance critical by nature.

[Scanf-s]

@natemort

Hi, I'm interested in working on this issue and would like to contribute!

Before I get started, I briefly investigated and found that tokenbucket is used in two places.

• In the CLI, it uses only the normal behavior of TokenBucket, so it seems straightforward to remove and refactor it using common/clock/ratelimiter.go.

• But there is a complexity with common/persistence/wrappers/sampled. Because there is no priority implementation in common/clock/ratelimiter.go. And I think this would make another problem if I just remove common/tokenbucket.

So should I just refactor the CLI without removing tokenbucket and using common/clock/ratelimiter.go?

This is my first contribute to Cadence, so I just want to make sure that I'm going in the right decision!

[natemort]

Hey @Scanf-s , thanks for digging into that further. I wasn't familiar with the visibility sampling functionality and missed that it uses the TokenBucket.

@neil-xie Any idea on why we have visibility sampling, whether we need it, and whether it needs the prioritization functionality?

I think removing the tokenbucket from the CLI would be useful as a first step and then maybe we follow that up with removing the tokenbucket if possible.