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

Concurrency issues #613

Closed
andyleap opened this Issue Nov 7, 2014 · 17 comments

Comments

3 participants
@andyleap
Contributor

andyleap commented Nov 7, 2014

Something is making the application return 404's randomly when refreshing too fast. The errors happen all over the place, even at the home url "/". The issue seems to be that something can't handle concurrent access, as when I wrap the macaron http handler in a func like

var mutex sync.Mutex

handler := func (w http.ResponseWriter, req *http.Request) {
        mutex.Lock()
        m.ServeHTTP(w, req)
        mutex.Unlock()
}

and pass that like http.ListenAndServe(listenAddr, http.HandlerFunc(handler)), everything works fine.

Rather serious issue if the system can't handle multiple requests at once.....

@Unknwon

This comment has been minimized.

Show comment
Hide comment
@Unknwon

Unknwon Nov 7, 2014

Member

Thanks your feedback!

But I think the change you made just make macaron can only handle 1 request at a time? Not sure.

I've did ab test on 1000 connections and 10000 requests and didn't seem to have this random situation, so can you provide more information?

Member

Unknwon commented Nov 7, 2014

Thanks your feedback!

But I think the change you made just make macaron can only handle 1 request at a time? Not sure.

I've did ab test on 1000 connections and 10000 requests and didn't seem to have this random situation, so can you provide more information?

@andyleap

This comment has been minimized.

Show comment
Hide comment
@andyleap

andyleap Nov 7, 2014

Contributor

I just put compiled a -race build and there are data races ALL OVER THE PLACE. There's some seriously bad stuff going on here.

Contributor

andyleap commented Nov 7, 2014

I just put compiled a -race build and there are data races ALL OVER THE PLACE. There's some seriously bad stuff going on here.

@Unknwon

This comment has been minimized.

Show comment
Hide comment
@Unknwon

Unknwon Nov 7, 2014

Member

Good point, haven't tried -race flag before. BTW, is this should be considered as Gogs issue or Macaron?

Member

Unknwon commented Nov 7, 2014

Good point, haven't tried -race flag before. BTW, is this should be considered as Gogs issue or Macaron?

@andyleap

This comment has been minimized.

Show comment
Hide comment
@andyleap

andyleap Nov 7, 2014

Contributor

There's data races in the cron, in the orm, in the captcha, and that's just what I've seen in 5 minutes. I'd say it's an everywhere issue...

Contributor

andyleap commented Nov 7, 2014

There's data races in the cron, in the orm, in the captcha, and that's just what I've seen in 5 minutes. I'd say it's an everywhere issue...

@Unknwon

This comment has been minimized.

Show comment
Hide comment
@Unknwon

Unknwon Nov 7, 2014

Member

In my point of view, as long as no deadlock..data race is unavoidable.

Member

Unknwon commented Nov 7, 2014

In my point of view, as long as no deadlock..data race is unavoidable.

@andyleap

This comment has been minimized.

Show comment
Hide comment
@andyleap

andyleap Nov 7, 2014

Contributor

I really don't mean to be rude, but they are hardly unavoidable, and while not causing deadlocks, they are causing issues. I'm not sure I can use this software as it is.

Contributor

andyleap commented Nov 7, 2014

I really don't mean to be rude, but they are hardly unavoidable, and while not causing deadlocks, they are causing issues. I'm not sure I can use this software as it is.

@Unknwon

This comment has been minimized.

Show comment
Hide comment
@Unknwon

Unknwon Nov 7, 2014

Member

If to use things like sync.Mutex, I think there is really no concurrent at all, just a single thread..

Member

Unknwon commented Nov 7, 2014

If to use things like sync.Mutex, I think there is really no concurrent at all, just a single thread..

@andyleap

This comment has been minimized.

Show comment
Hide comment
@andyleap

andyleap Nov 7, 2014

Contributor

and if I don't, I get 404's on explore, dashboard, user profiles, and more. Think I'll take single thread over that.

Contributor

andyleap commented Nov 7, 2014

and if I don't, I get 404's on explore, dashboard, user profiles, and more. Think I'll take single thread over that.

@Unknwon

This comment has been minimized.

Show comment
Hide comment
@Unknwon

Unknwon Nov 7, 2014

Member

What is your system and version of Gogs? And is it possible to reproduce on https://try.gogs.io?

Member

Unknwon commented Nov 7, 2014

What is your system and version of Gogs? And is it possible to reproduce on https://try.gogs.io?

@andyleap

This comment has been minimized.

Show comment
Hide comment
@andyleap

andyleap Nov 7, 2014

Contributor

0.5.7.1105 Beta on linux. Tried ab against it compiled without the mutex, and gogs crashed.

Contributor

andyleap commented Nov 7, 2014

0.5.7.1105 Beta on linux. Tried ab against it compiled without the mutex, and gogs crashed.

@andyleap

This comment has been minimized.

Show comment
Hide comment
@andyleap

andyleap Nov 7, 2014

Contributor

With roughly 99,950 or so lines of stack traces and an error of too many open files.

Contributor

andyleap commented Nov 7, 2014

With roughly 99,950 or so lines of stack traces and an error of too many open files.

@Unknwon

This comment has been minimized.

Show comment
Hide comment
@Unknwon

Unknwon Nov 7, 2014

Member

0.5.7.1105 Beta on linux. Tried ab against it compiled without the mutex, and gogs crashed.

What is the behave of crashed of Gogs?

With roughly 99,950 or so lines of stack traces and an error of too many open files.

This is limited by your ulimit -n.

Member

Unknwon commented Nov 7, 2014

0.5.7.1105 Beta on linux. Tried ab against it compiled without the mutex, and gogs crashed.

What is the behave of crashed of Gogs?

With roughly 99,950 or so lines of stack traces and an error of too many open files.

This is limited by your ulimit -n.

@lifeofguenter

This comment has been minimized.

Show comment
Hide comment
@lifeofguenter

lifeofguenter Nov 14, 2014

Is this maybe the reason why sometimes the avatar won't load?

-> I have avatar caching activated
-> on a site where a lot of avatars are being loaded (logged in -> dashboard)
-> avatar cache is being refreshed

Result:
-> some avatars (or all) do not load.

Also some improvements I would make with the avatar caching:

  1. ability to configure the TTL, per default it can be at least a day (though thinking that even 7-30 days is ok)
  2. add an expire header with the same value of the TTL, so client does not need to download the avatar each time
  3. fix the ETAG (currently it is "Etag:size(80)"

lifeofguenter commented Nov 14, 2014

Is this maybe the reason why sometimes the avatar won't load?

-> I have avatar caching activated
-> on a site where a lot of avatars are being loaded (logged in -> dashboard)
-> avatar cache is being refreshed

Result:
-> some avatars (or all) do not load.

Also some improvements I would make with the avatar caching:

  1. ability to configure the TTL, per default it can be at least a day (though thinking that even 7-30 days is ok)
  2. add an expire header with the same value of the TTL, so client does not need to download the avatar each time
  3. fix the ETAG (currently it is "Etag:size(80)"
@Unknwon

This comment has been minimized.

Show comment
Hide comment
@Unknwon

Unknwon Nov 14, 2014

Member

@lifeofguenter thanks your info! I'll take a look ASAP.

Member

Unknwon commented Nov 14, 2014

@lifeofguenter thanks your info! I'll take a look ASAP.

@andyleap

This comment has been minimized.

Show comment
Hide comment
@andyleap

andyleap Mar 4, 2015

Contributor
    With roughly 99,950 or so lines of stack traces and an error of too many open files.

This is limited by your ulimit -n.

Simple load test shouldn't kill the server due to open file limits, that's insane.

Is this maybe the reason why sometimes the avatar won't load?

-> I have avatar caching activated
-> on a site where a lot of avatars are being loaded (logged in -> dashboard)
-> avatar cache is being refreshed

Result:
-> some avatars (or all) do not load.

Yeah, I was having this issue to, tracked it down to the server randomly throwing 404's on the avatar urls. Disabling concurrency fixed it right away.

Contributor

andyleap commented Mar 4, 2015

    With roughly 99,950 or so lines of stack traces and an error of too many open files.

This is limited by your ulimit -n.

Simple load test shouldn't kill the server due to open file limits, that's insane.

Is this maybe the reason why sometimes the avatar won't load?

-> I have avatar caching activated
-> on a site where a lot of avatars are being loaded (logged in -> dashboard)
-> avatar cache is being refreshed

Result:
-> some avatars (or all) do not load.

Yeah, I was having this issue to, tracked it down to the server randomly throwing 404's on the avatar urls. Disabling concurrency fixed it right away.

@Unknwon

This comment has been minimized.

Show comment
Hide comment
@Unknwon

Unknwon Mar 4, 2015

Member

@andyleap thanks for the update!

Member

Unknwon commented Mar 4, 2015

@andyleap thanks for the update!

@Unknwon

This comment has been minimized.

Show comment
Hide comment
@Unknwon

Unknwon Nov 13, 2015

Member

The primary race problem should have been fixed by Macaron itself. So close this now.

Member

Unknwon commented Nov 13, 2015

The primary race problem should have been fixed by Macaron itself. So close this now.

@Unknwon Unknwon closed this Nov 13, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment