Skip to content
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

How does one minimize server restart? #787

Closed
gedion opened this issue Jun 14, 2012 · 21 comments
Closed

How does one minimize server restart? #787

gedion opened this issue Jun 14, 2012 · 21 comments

Comments

@gedion
Copy link
Contributor

gedion commented Jun 14, 2012

Certain user inputs seem to break a pad and cause the shutdown of a server.

How can you minimize this? If a single pad breaks it should not affect other users. The server should be able to go on for other pads.

@jhollinger
Copy link
Contributor

True, nothing like that should crash the server. I'm sure that can be prevented, but first, what kind of user input is causing problems?

@fourplusone
Copy link
Contributor

I think mostly bad formatted changesets (like #752), messages with missing attributes and sometimes hard to reproduce race conditions (I think most of them happen while iteration async throu an array).

@mrneil
Copy link
Contributor

mrneil commented Jun 18, 2012

I agree with @fourplusone. And I would add unstable connections to that list.

We've been running Etherpad Lite for almost four months and have over 775,000 revisions to a single pad. One of the users is mobile most of the time and therefore doesn't have the most stable Internet connection.

When applicable after each crash, I've added some lines to PadMessageHandler.js that simply make sure a variable/object (usually sessioninfos[]) is not undefined before accessing any of its properties or functions. My theory is these aren't set because a connection has dropped while an async iteration (or whatever) is running.

To be honest, identical failures happen infrequently and I'm only aware of a single instance where one of my tests prevented a shutdown. But if these changes pan out over time, I'll be adding them to the project. (Of course, I'm happy to share if anyone wants them now.)

@marcelklehr
Copy link
Contributor

Why not? Just open a pull request with your changes and we'll see, what we'll do with them. Thanks.

@mrneil
Copy link
Contributor

mrneil commented Jul 6, 2012

I haven't added them yet because: (a) they're still undergoing basic testing, and (b) I haven't completely learned git and pull requests at this time.

As I wrote though, once I overcome those two problems, the changes will be added to the project.

@gedion
Copy link
Contributor Author

gedion commented Aug 8, 2012

Here is one common restart. Even though our app logs you out and redirects after 1 hour, could it be a session expire error?

[31m[2012-08-05 04:24:23.140] [ERROR] console -[39mTypeError: Cannot set property 's.5FBDR9wWnIIbyGZY' of undefined
at Object.callback (/home/src/node/db/SessionManager.js:148:46)
at /home/src/node_modules/ueberDB/CloneAndAtomicLayer.js:95:17
at /home/src/node_modules/ueberDB/CacheAndBufferLayer.js:183:7
RESTART!

@marcelklehr
Copy link
Contributor

No, that's probably a mere bug:

This should fix it:

@ line 141 in SessionManager.js
-        if(group2sessions == null)
+        if(group2sessions == null || group2sessions.sessionIDs == null)
         {
           group2sessions = {sessionIDs : {}};
         }

(I'll submit a pr tomorrow...)

@gedion
Copy link
Contributor Author

gedion commented Aug 8, 2012

Thanks for the quick response. Users do not like the server restarts. I think that's the only issue that's keeping our app from taking a flight.

@marcelklehr
Copy link
Contributor

Another question: Which git revision are you running?

@gedion
Copy link
Contributor Author

gedion commented Aug 9, 2012

My ep lite version is 1.0.0

@marcelklehr
Copy link
Contributor

Did you know, the latest version is 1.1.1?

Anyway, trusting git-blame this isn't fixed in latest develop, so: try #939

@gedion
Copy link
Contributor Author

gedion commented Aug 10, 2012

Hmm I thought I had the latest. We're using plugins and everything already. I'm not trusting the output that says my version 1.0.0. I think I may have used develop version. I'll update it to the latest.

Anyway. With that fix you suggested, we now see similar error but in a different place.

[31m[2012-08-09 18:33:56.479] [ERROR] console - [39mTypeError: Cannot set property 's.QLKy2fmwA4CvesoT' of undefined
    at Object.callback (/home/src/node/db/SessionManager.js:171:47)
    at /home/src/node_modules/ueberDB/CloneAndAtomicLayer.js:95:17
    at /home/src/node_modules/ueberDB/CacheAndBufferLayer.js:183:7

@marcelklehr
Copy link
Contributor

Ah, well... I guess you have to change this, too :)

@ line 165 in src/node/db/SessionManager.js
-        if(author2sessions == null)
+        if(author2sessions == null || author2sessions.sessionIDs == null)
        {
          author2sessions = {sessionIDs : {}};
        }

@marcelklehr
Copy link
Contributor

Updated #939...

@marcelklehr
Copy link
Contributor

Any other errors that cause server restarts?

@gedion
Copy link
Contributor Author

gedion commented Sep 2, 2012

Hi marcelklehr. No it's been great since your fix : ). Our users(in Ireland) create about 15 pads a day. Since then we only saw one restart and it was because of our database. We are moving on to a 2nd and larger user base located here in the US. Initial feedback from a small group has been positive. Thanks for the fix. You saved my life. It's so unfair how a small bug like this can be a show stopper.

@gedion gedion closed this as completed Sep 2, 2012
@marcelklehr
Copy link
Contributor

Very good news!

@gedion
Copy link
Contributor Author

gedion commented Sep 11, 2012

A couple of new ones showed up.

This first one occurred a week or so ago but have not seen it since.

 [31m[2012-08-30 03:22:25.553] [ERROR] console -  [39mAsync Stacktrace:
    at src/node/handler/APIHandler.js:159:7
    at src/node/db/API.js:220:8
    at src/node/db/API.js:552:8
    at Object.callback (src/node/db/PadManager.js:117:8)

SyntaxError: Unexpected end of input
    at Object.parse (native)
    at src/node_modules/ueberDB/CacheAndBufferLayer.js:162:24
    at [object Object].<anonymous> (/src/node_modules/ueberDB/oracle2_db.js:57:17)

The 2nd one occured today on two different pad

[31m[2012-09-11 05:11:10.928] [ERROR] console -  [39mError: exports: newlineEnd <= 0 in splitAttributionLines
    at Object.error (/src/static/js/Changeset.js:41:11)
    at Object.assert (/src/static/js/Changeset.js:55:13)
    at Object.splitAttributionLines (/src/static/js/Changeset.js:1330:15)
    at getHTMLFromAtext (/src/node/utils/ExportHtml.js:99:31)
    at /src/node/utils/ExportHtml.js:80:12
    at /src/node_modules/async/lib/async.js:517:34
    at Array.0 (/src/node_modules/async/lib/async.js:441:34)
    at EventEmitter._tickCallback (node.js:192:40)

@gedion gedion reopened this Oct 3, 2012
@gedion
Copy link
Contributor Author

gedion commented Oct 3, 2012

Here is a new one I've not seen. It happened 3 times today.
Is it possible to print out the pad id that caused the error?

at /src/node/handler/APIHandler.js:159:7
    at /src/node/db/API.js:220:8
    at /src/node/db/API.js:552:8
    at Object.callback (/src/node/db/PadManager.js:117:8)

SyntaxError: Unexpected end of input
    at Object.parse (native)
    at /src/node_modules/ueberDB/CacheAndBufferLayer.js:162:24    

@marcelklehr
Copy link
Contributor

Git revision? :)

@gedion
Copy link
Contributor Author

gedion commented Oct 5, 2012

Same as before. Not quiet ready to update with the latest epLite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants