Refactor counter implementations to be polymorphic
What does this MR do and why?
This is a refactor MR to make increments on project statistics easier to follow. There is no change to any apparent behaviour. This MR sets up the groundwork for the following MR to fix the counter refresh (still in draft). !104216 (diffs)
-
#increment_counteris now defined withinCounterAttribute. It will toggle between the legacy ActiveRecord counter and the Redis backed counter, now calledBufferedCounter, based on the defined list ofcounter_attributes. - Add 2 new classes to encapsulate increment operations:
-
Gitlab::Counters::BufferedCounteris the Redis backed counter, with most of the Redis implementation contained in this class. There are still some left inCounterAttribute. This change will follow. -
Gitlab::Counters::LegacyCounteris a wrapper around the legacy ActiveRecord counter. It simply delegates back to the ActiveRecord.
-
As a result, from the perspective of a client of ProjectStatistics such as various services, there is no need to distinguish how a counter for a particular attribute is implemented, be it Redis or ActiveRecord.
This is also needed so that more complex Redis implementation needed to fix https://gitlab.com/gitlab-org/gitlab/-/issues/373469 can be contained within the Redis counter and be more easily tested.
Follow ups to be done in following MRs
-
clean up remaining counter_attribute_enabled?in various methods. This enabled check should only be needed when deciding which counter to use. -
avoid exposing Redis keys to flush_increments_to_database!if possible.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #373469