increment hits instead of adding new row

[Rubioli]

punching_bag gem is a great. But when it comes to performance I have some worries since its adds a new row for each hit.

I was wondering if its possible to increment by 1 if punchable_id exists already otherwise add a new row. This will help performance much.

I know I can do rake punching_bag:combine but then I need to to crone jobs etc and at the same time when I run rake punching_bag:combine if some posts are removed, its aborting the task since it can't find them. This is the errors:

rake aborted!
ActiveRecord::RecordNotFound: Couldn't find all Posts with 'id': (1,3,5,7,8,9)

Is it any way I can change/where can I change the gem to increment and not always add a new line?

[Rubioli]

If someone want the fix, comment here and I will post it

[tmartin8080]

@Rubioli I'm interested in this fix, or can you create a PR?

[adamcrown]

Sorry for the very long delay in getting to this. I've got some more urgent issues with this gem to attend to at the moment, but I'll keep this issue open and see if I can improve the performance in some way.

I'd honestly, like to see a comparison of the performance in updating a row, vs inserting one. It's been a long time since I wrote this gem, or even worked with it. But my original thinking was that it would be better to insert a row to limit the number of queries and calculations. But I'm open to other, better ways.

[MyklClason]

I'm about to start using this gem so reviewed the issues. I think I have a suitable solution to this, though not in the direction that Rubioli was looking.

The issue is that the scenario Rubioli mentions is basically only relevant if the web app's performance is bottle-necked by the database, this is a situation that is relatively rare (aside from simply using inefficient database queries). I imagine this is only relevant in a situation where there are dozens if not hundreds of hits per second.

I think there is a better, more efficient approach. Use redis or something to allow the count to persist and be updated across requests and servers, then add all the hits that occur during X amount of time in a single row via a background process. However, again, the cases where this is needed are very rare indeed and I don't think it's practical to implement that level of complexity into the gem, as such I would instead propose:

Using something like @post.punch(request, times: 5) to add a new row that already has been hit 5 times, by default, all rows will only have been hit once. This allows the developer to work out a top level counter handles combining the counts for a < 1-5 seconds interval before they are passed to punching_bag. Also, I would also make this an optional migration/functionality so the functionality is only used when it's needed, this minimizes the performance impact for those who don't need it.

[MyklClason]

Huh was just reading the doc and seems you can already pass the count (times, as of late 2015), in which case, I don't see anything preventing you from simply doing as I mention and caching the hits if you have a high rate of punches in a short time, if creating new rows actually becomes a performance issue. I don't see this is something Punching bag needs to solve itself.

https://github.com/biola/punching_bag/blob/master/lib/punching_bag.rb#L6

punching_bag/lib/punching_bag.rb

Line 6 in dd5b60c

def self.punch(punchable, request = nil, count = 1)

dd5b60c#diff-6b8394add13d00d3bbd56bbd83d3fbec