coralproject / pillar

Deprecated: Service layer for the Coral ecosystem
Other
4 stars 1 forks source link

Leave Search events should fire when search comes back empty #138

Closed jde closed 8 years ago

jde commented 8 years ago

Expected behavior

When a search comes back empty, pillar should trigger an event for each user that was in the group saying that the user left the search.

Actual behavior

Pillar exits doing nothing https://github.com/coralproject/pillar/blob/master/pkg/service/search_cron.go#L102

Steps to reproduce behavior

alexbyrnes commented 8 years ago

Some notes as I find them. I might need some clarification for this ticket. As I recall we were looking for a message that should have been logged when pillar needs to update the tags on a user. Now we have statsd running so we should be able to trace the event more easily.

The line number refers to a different line in the version of search_cron now. Line 102 is a comment line. The line we're looking for is possibly line 39.

In statsd I am getting some Tag_Removed events, which makes me think it is implemented but something else is preventing the events from firing.

I am seeing an error with a current search that has an empty string id:

https://github.com/coralproject/pillar/blob/master/pkg/service/search_cron.go#L114


2016/07/26 20:00:34 cron: panic running job: Invalid input to ObjectIdHex: ""
goroutine 80004 [running]:
github.com/coralproject/pillar/vendor/github.com/robfig/cron.(*Cron).runWithRecovery.func1(0xc8201cc040)
    /home/ec2-user/deploy/go/src/github.com/coralproject/pillar/vendor/github.com/robfig/cron/cron.go:139 +0xa6
github.com/coralproject/pillar/vendor/gopkg.in/mgo.v2/bson.ObjectIdHex(0x0, 0x0, 0x0, 0x0)
    /home/ec2-user/deploy/go/src/github.com/coralproject/pillar/vendor/gopkg.in/mgo.v2/bson/bson.go:175 +0x210
github.com/coralproject/pillar/pkg/service.getNewUsers(0xc8202006a0, 0xc820203bc0, 0xc, 0xc8204a3760, 0x16, 0x0, 0x0, 0xc820204330, 0x25, 0x0, ...)
    /home/ec2-user/deploy/go/src/github.com/coralproject/pillar/pkg/service/search_cron.go:114 +0x196
github.com/coralproject/pillar/pkg/service.doUpdateSearch(0xc820389f20, 0xc820203bc0, 0xc, 0xc8204a3760, 0x16, 0x0, 0x0, 0xc820204330, 0x25, 0x0, ...)
    /home/ec2-user/deploy/go/src/github.com/coralproject/pillar/pkg/service/search_cron.go:33 +0x58
github.com/coralproject/pillar/pkg/service.UpdateSearch()
    /home/ec2-user/deploy/go/src/github.com/coralproject/pillar/pkg/service/search_cron.go:27 +0x2ee
github.com/coralproject/pillar/vendor/github.com/robfig/cron.FuncJob.Run(0x9cf958)
    /home/ec2-user/deploy/go/src/github.com/coralproject/pillar/vendor/github.com/robfig/cron/cron.go:87 +0x19
github.com/coralproject/pillar/vendor/github.com/robfig/cron.(*Cron).runWithRecovery(0xc8201cc040, 0x7f43471a39c8, 0x9cf958)
    /home/ec2-user/deploy/go/src/github.com/coralproject/pillar/vendor/github.com/robfig/cron/cron.go:143 +0x4e
created by github.com/coralproject/pillar/vendor/github.com/robfig/cron.(*Cron).run
    /home/ec2-user/deploy/go/src/github.com/coralproject/pillar/vendor/github.com/robfig/cron/cron.go:175 +0x589
alexbyrnes commented 8 years ago

Panic occurs 34 seconds past the minute every minute.

2016/07/29 03:21:34 cron: panic running job: Invalid input to ObjectIdHex: "" 2016/07/29 03:22:34 cron: panic running job: Invalid input to ObjectIdHex: "" 2016/07/29 03:23:34 cron: panic running job: Invalid input to ObjectIdHex: "" 2016/07/29 03:24:34 cron: panic running job: Invalid input to ObjectIdHex: "" 2016/07/29 03:25:34 cron: panic running job: Invalid input to ObjectIdHex: ""

So each search cron has an error. I assume this was fixed by omit empty for bson.IDs in the models.

@jde I'd like to do an upgrade to rule this out. Please ring in if that's not an option.

jde commented 8 years ago

@alexbyrnes Good find. I'm not clear on the upgrade. The commit you refer to looks to be already in master. Is there code in a branch I can look at?

alexbyrnes commented 8 years ago

I'm wondering if I can upgrade pillar to see if it's more stable.

jde commented 8 years ago

Ah. Yes. Feel free to deploy the latest version of Pillar if it will help. I would recommend setting up a development environment to figure out issues like this, however. Trying things like this out on a staging will lead to a lot of usage issues.

alexbyrnes commented 8 years ago

Dev instance is up. I think there was some confusion over whether you were talking about a local dev server or a dev instance. We definitely need another instance so we can test things that everyone can see and verify.

The newest pillar has been fine so far. The bug #180 is not fixed. Gaba's fix was for Ask. search_cron.go was modified as part of that but not for this purpose. My mistake. I saw "omit empty for bson.IDs" in the history for search_cron and jumped to the wrong conclusion. I put an if in there which is not ideal but it fixes the problem for now or until #182 shakes things up.

alexbyrnes commented 8 years ago

By the way, I'm not implying #182 closes this ticket. This functionality will follow the code into xenia.

jde commented 8 years ago

Sounds good. I know it differs from org to org. Internally at coral we've all gravitated toward:

alexbyrnes commented 8 years ago

Staging I'm thinking of as the live or closest we have to live and dev is for internal tests. Presumably prod will be coming soon.

jde commented 8 years ago

That way of describing environments isn't clear to me. When interacting as a member of Coral, please use the standard environment concepts outlined above. They are pretty universal across the industry.

alexbyrnes commented 8 years ago

I feel like I am using industry standards. The nonstandard part is calling staging live, which is a quirk we've been living with for a while. I will definitely try to be clear, that said. Appreciate the feedback.