• DonaldK (unregistered)

    And here I thought MQ was supposed to elegantly queue entries so that you don't have hurricane issues...

    Frist BTW

  • On a Break (unregistered) in reply to DonaldK

    The thread-pool library I use does that. The first time it's called it spawns however many threads seem sane given the underlying hardware.

    Hand it stacks of Runnables or method/parameter/result_placeholder lists and it wakes up the threads does them.

    When the threads run out of stuff to do they go to sleep, ready for the next time.

    I don't care how many threads there actually are (though I can find out or specify if I really want to), I just make a thread-pool and let it handle it. I don't know if it dynamically resizes the pool if CPUs magically appear during runtime, but given the hardware I target this won't happen so I really don't care.

  • csrster (unregistered)

    Bleurgh.

    I've always use commons-pool for this sort of thing: http://commons.apache.org/proper/commons-pool/

  • foxyshadis (unregistered) in reply to On a Break
    On a Break:
    The thread-pool library I use does that. The first time it's called it spawns however many threads seem sane given the underlying hardware.

    Hand it stacks of Runnables or method/parameter/result_placeholder lists and it wakes up the threads does them.

    When the threads run out of stuff to do they go to sleep, ready for the next time.

    I don't care how many threads there actually are (though I can find out or specify if I really want to), I just make a thread-pool and let it handle it. I don't know if it dynamically resizes the pool if CPUs magically appear during runtime, but given the hardware I target this won't happen so I really don't care.

    This isn't even a threadpool, it's a very poorly implemented spinlock. They might as well not even have threads anymore, it's just a very weird way to make a queue now.

  • (cs)
    Of course, since he was the apex predator, nobody would (could) review his code
    Really? Really??? I'm not going to "TRWTF is..." but wherever I've worked (big or small), the top fish have never been so arrogant as to not request a junior to sanity check (or "formally review") their code if there's no one above them.
  • faoileag (unregistered) in reply to On a Break
    On a Break:
    The thread-pool library I use does that.
    Does it really? Or did you miss the last sentence?

    This is not a thread-pool. It's a message-queue emptying system that will get into an endless sleep if ever inserting a message fails, leaving the remaining messages forever on hold.

  • faoileag (unregistered)

    There's a second WTF in there as well - the InboundMessageHandler will also skip the exit condition of the while loop and happily sleep for another 10ms if getNumberOfDataRows() ever throws an exception querying the number of rows.

    Mandatory cartoon: http://geekandpoke.typepad.com/geekandpoke/2009/06/simply-explained-checked-exceptions.html

    No Akismet this is not spam.

  • erica (unregistered) in reply to On a Break

    erica a linda

  • gtg (unregistered)

    A pleasingly compound WTF.

    1. Master Architect's code not reviewed
    2. His/her change has made the message handler effectively single threaded
    3. Hangs if an incoming message doesn't add rows to the DB (either by error or intentionally)
    4. Comments to terminate every code block
  • (cs)

    this is so much easier to do by just using a thread pool as available in the standard java library

  • threadbear (unregistered)

    Aha, the classic "I've read about how to do multithreading so I'm now going to implement my own multithreaded solution " WTF.

    Seen this sort of thing many times, especially on Enterprise Java systems, and it invariably causes problems like this, despite the fact that

    a) Java app servers handle request / response processing in a multithreaded manner by design in order to support a large volume of traffic, so kicking off your own threads when handling requests is generally stupid and pointless.

    b) As pointed out by other posters here, there are several free and robust messaging systems available on the platform, and not using one of them is classic "not invented here" stupidity

    c) multithreaded systems are notoriously prone to these type of problems when developers armed with their newly acquired multithreading hammer start to see everything as a nail.

  • (cs)

    And this is Java 7 code, so the excuse "but it's old code" doesn't work either.

  • (cs)

    A number of things occur to me.

    • We don't know how InboundMessageHandler is used, so comments about single-threaded operation aren't necessarily accurate. The fact that it extends Thread suggests that they might create several of them that all read from the same queue. This had better be the case because...

    • Each instance of InboundMessageHandler can handle at most 100 messages per second because of the sleep, except in bizarre cases where the worker thread that gets spawned completes before the first pass through the while() header.

    • The original problem "merely" needed a cap on the number of threads, nothing more. (I once worked with a Win32 codebase that used a Microsoft framework for handling media packet flows. This framework used one thread per flow, and our application (a VoIP server/gateway) needed several flows per VoIP call. It gave up accepting calls when the total flow load was not far above 1500 flows, and consequently 1500 flow-handler threads, and consequently 1500MB of stack space. Given all the other stuff in memory, those >1500 1MB thread stacks wouldn't all fit... Don't do it this way, boys and girls.)

  • (cs) in reply to threadbear
    threadbear:
    c) multithreaded systems are notoriously prone to these type of problems when developers armed with their newly acquired multithreading hammer start to see everything as a singlethreaded nail.

    wooo... for fun.

  • Taemyr (unregistered) in reply to Steve The Cynic
    Steve The Cynic:
    * We don't know how InboundMessageHandler is used, so comments about single-threaded operation aren't necessarily accurate. The fact that it extends Thread suggests that they might create several of them that all read from the same queue.
    More than one of these handlers causes a problem. Unless each thread reads from it's own db. Because the MessageHandler assumes that it's worker has finished when the number of rows have increased. - Of course there could be lock on the db somewhere, but then at most one handler will operate at a time.
  • faoileag (unregistered) in reply to Taemyr
    Taemyr:
    More than one of these handlers causes a problem. Unless each thread reads from it's own db.
    I think that possibility can be dismissed, as the connection data seems to be hardcoded into ThrottleDaoImpl; so all ThrottleDaoImpl instances not only access the same database, they also operate on the same table.
  • faoileag (unregistered)

    One more interesting thought about the system. The article says "in one particular system, the data was coming in way too fast", resulting in so many threads, that the number of threads exceeded the system max.

    Now the article does not say whether the data was coming in continuously at a very fast rate or rather in bursts; but if the data is coming in continuously at a very fast rate, than the system will sooner or later face another problem, i.e. that the message queue will grow over time.

    If so much data is coming in that immediately assigning a thread for each message will run out of threads, then slowing down the message handling process might prevent the system from running out of threads.

    But messages will also get handled later and later with the message queue growing; I don't think that this is a sane situation for the system as well.

  • (cs) in reply to Zagyg
    Zagyg:
    Of course, since he was the apex predator, nobody would (could) review his code
    Really? Really??? I'm not going to "TRWTF is..." but wherever I've worked (big or small), the top fish have never been so arrogant as to not request a junior to sanity check (or "formally review") their code if there's no one above them.
    So based on your sample size of 1...
  • (cs)

    Have these guys hearing of TPL?

  • Arggghhh (unregistered) in reply to Zagyg
    Zagyg:
    Of course, since he was the apex predator, nobody would (could) review his code
    Really? Really??? I'm not going to "TRWTF is..." but wherever I've worked (big or small), the top fish have never been so arrogant as to not request a junior to sanity check (or "formally review") their code if there's no one above them.

    Oh yeah, the junior can "formally review it" for sure. But she isn't going to be able to provide any criticism beyond the trivial -- it's just a rubber stamp.

    Captach: odio -- she'll have to approve the code no matter how odio it is.

  • emaN ruoY (unregistered) in reply to Zagyg
    Zagyg:
    Of course, since he was the apex predator, nobody would (could) review his code
    Really? Really??? I'm not going to "TRWTF is..." but wherever I've worked (big or small), the top fish have never been so arrogant as to not request a junior to sanity check (or "formally review") their code if there's no one above them.
    Welcome to The Daily WTF, where most stories involving code are either performed by juniors or seniors, were the experienced worker has no time or no influential power to review code that ends up in production. And a QA check may or may not catch a thread overrun. (If they even have a QA team/person.)
  • Anon 6848946 (unregistered) in reply to threadbear
    threadbear:
    a) Java app servers handle request / response processing in a multithreaded manner by design in order to support a large volume of traffic, so kicking off your own threads when handling requests is generally stupid and pointless.

    This is only true when the actions performed are lightweight. If an operation requires multiple seconds or more to finish, the system will feel very unresponsive.

    This of course does not necessarily mean you should implement parallel processing yourself, or that you even need a new thread. At least with servlet 3.0 there's some other ways to do this in a cleaner and more efficient manner.

  • (cs) in reply to Taemyr
    Taemyr:
    Steve The Cynic:
    * We don't know how InboundMessageHandler is used, so comments about single-threaded operation aren't necessarily accurate. The fact that it extends Thread suggests that they might create several of them that all read from the same queue.
    More than one of these handlers causes a problem. Unless each thread reads from it's own db. Because the MessageHandler assumes that it's worker has finished when the number of rows have increased. - Of course there could be lock on the db somewhere, but then at most one handler will operate at a time.
    Not really, because we only look for "increased", not "increased by X". Each thread that is in here in parallel will post a request that will increase the row count by X (presumably 1) and then wait for the request to complete. If there are several in parallel, they'll all see an increase of (several*X) at the same moment, 10 ms later...
  • foo AKA fooo (unregistered) in reply to D-Coder
    D-Coder:
    Zagyg:
    Of course, since he was the apex predator, nobody would (could) review his code
    Really? Really??? I'm not going to "TRWTF is..." but wherever I've worked (big or small), the top fish have never been so arrogant as to not request a junior to sanity check (or "formally review") their code if there's no one above them.
    So based on your sample size of 1...
    Are you suggesting he worked at Schrödinger's?

    (Of course, the top fish wouldn't have been alive unless that atom decayed ...)

  • (cs) in reply to Arggghhh
    Arggghhh:
    Zagyg:
    Of course, since he was the apex predator, nobody would (could) review his code
    Really? Really??? I'm not going to "TRWTF is..." but wherever I've worked (big or small), the top fish have never been so arrogant as to not request a junior to sanity check (or "formally review") their code if there's no one above them.

    Oh yeah, the junior can "formally review it" for sure. But she isn't going to be able to provide any criticism beyond the trivial -- it's just a rubber stamp.

    Captach: odio -- she'll have to approve the code no matter how odio it is.

                      try {
                          Thread.sleep(10); // 10 milliseconds
                      } catch (InterruptedException ie) {
                        // do nothing
                      }
    

    That would have been an immediate red flag for me, no matter what my level of experience is.

  • foo AKA fooo (unregistered) in reply to chubertdev
    chubertdev:
                      try {
                          Thread.sleep(10); // 10 milliseconds
                      } catch (InterruptedException ie) {
                        // do nothing
                      }
    

    That would have been an immediate red flag for me, no matter what my level of experience is.

    ... which seems to be on the level of "smallowing exceptions = always bad". With a higher level of experience, you might realize someday that this isn't an absolute rule. In this case, the exception is that a sleep was interrupted; furthermore the sleep is used in a retry loop. Therefore, cutting the sleep short will do no more harm than retrying a little earlier. (This is not meant to suggest that the code is sane in any other way, of course, but if you have a valid case for a retry-sleep loop, ignoring short sleeps is usually not a problem. In fact, any effort to complete the sleep exactly is usually unnecessary complexity and therefore bad.)

  • Brompot (unregistered) in reply to chubertdev
    chubertdev:
    Arggghhh:
    Zagyg:
    Of course, since he was the apex predator, nobody would (could) review his code
    Really? Really??? I'm not going to "TRWTF is..." but wherever I've worked (big or small), the top fish have never been so arrogant as to not request a junior to sanity check (or "formally review") their code if there's no one above them.

    Oh yeah, the junior can "formally review it" for sure. But she isn't going to be able to provide any criticism beyond the trivial -- it's just a rubber stamp.

    Captach: odio -- she'll have to approve the code no matter how odio it is.

                      try {
                          Thread.sleep(10); // 10 milliseconds
                      } catch (InterruptedException ie) {
                        // do nothing
                      }
    

    That would have been an immediate red flag for me, no matter what my level of experience is.

    Most of the time they say:

    // TODO: Implement exception handling

    Not a red flag for a junior, it will be implemented, right?

  • blakeey rat (unregistered)

    i realy dont like this website

  • (cs) in reply to foo AKA fooo
    foo AKA fooo:
    chubertdev:
                      try {
                          Thread.sleep(10); // 10 milliseconds
                      } catch (InterruptedException ie) {
                        // do nothing
                      }
    

    That would have been an immediate red flag for me, no matter what my level of experience is.

    ... which seems to be on the level of "smallowing exceptions = always bad". With a higher level of experience, you might realize someday that this isn't an absolute rule. In this case, the exception is that a sleep was interrupted; furthermore the sleep is used in a retry loop. Therefore, cutting the sleep short will do no more harm than retrying a little earlier. (This is not meant to suggest that the code is sane in any other way, of course, but if you have a valid case for a retry-sleep loop, ignoring short sleeps is usually not a problem. In fact, any effort to complete the sleep exactly is usually unnecessary complexity and therefore bad.)

    No, you at the very least log it. If you think otherwise, you have a very, very long way to go in your profession.

  • (cs) in reply to Brompot
    Brompot:
    Most of the time they say:

    // TODO: Implement exception handling

    Not a red flag for a junior, it will be implemented, right?

    //TODO: don't divide by zero here, the universe doesn't like that

  • Baboon (unregistered) in reply to foo AKA fooo

    Except it would always throw InterruptedException since he doesn't call Thread.interrupted() ...

  • Anonymous Cowherd (unregistered) in reply to foo AKA fooo
    foo AKA fooo:
    chubertdev:
                      try {
                          Thread.sleep(10); // 10 milliseconds
                      } catch (InterruptedException ie) {
                        // do nothing
                      }
    

    That would have been an immediate red flag for me, no matter what my level of experience is.

    ... which seems to be on the level of "smallowing exceptions = always bad". With a higher level of experience, you might realize someday that this isn't an absolute rule. In this case, the exception is that a sleep was interrupted; furthermore the sleep is used in a retry loop. Therefore, cutting the sleep short will do no more harm than retrying a little earlier. (This is not meant to suggest that the code is sane in any other way, of course, but if you have a valid case for a retry-sleep loop, ignoring short sleeps is usually not a problem. In fact, any effort to complete the sleep exactly is usually unnecessary complexity and therefore bad.)

    ... and with a slightly higher level of experience, you might realize someday that interrupting a thread is the standard way of asking a thread to clean up after itself and shut down, and the InterruptedException is how that request is received in the thread. So yes, while it may be okay to ignore some exceptions (NumberFormatException comes to mind, if you're sure about your inputs), this isn't really one of those cases.

  • foo AKA fooo (unregistered) in reply to chubertdev
    chubertdev:
    foo AKA fooo:
    chubertdev:
                      try {
                          Thread.sleep(10); // 10 milliseconds
                      } catch (InterruptedException ie) {
                        // do nothing
                      }
    

    That would have been an immediate red flag for me, no matter what my level of experience is.

    ... which seems to be on the level of "smallowing exceptions = always bad". With a higher level of experience, you might realize someday that this isn't an absolute rule. In this case, the exception is that a sleep was interrupted; furthermore the sleep is used in a retry loop. Therefore, cutting the sleep short will do no more harm than retrying a little earlier. (This is not meant to suggest that the code is sane in any other way, of course, but if you have a valid case for a retry-sleep loop, ignoring short sleeps is usually not a problem. In fact, any effort to complete the sleep exactly is usually unnecessary complexity and therefore bad.)

    No, you at the very least log it. If you think otherwise, you have a very, very long way to go in your profession.

    Spamming log files with things that are not errors is not professional but bureacratic.

    Just consider the situation: It will loop until some condition is met. The sleep is there just to avoid busy looping (which is a good thing, apart from the fact that the whole code is a bad idea in the first place). It doesn't matter at all whether or not there is an interruption during a sleep, i.e. if one sleep may last only 5ms instead of 10ms. The loop will finish within 10ms (+ scheduling overhead) of the condition being met either way. And the 10ms are arbitrary anyway, just an upper bound on the waiting time (which may be problematic as has been pointed out, but if 10ms is too long, an occasional shorter sleep is certainly not worse, possibly even a little better).

    So again: If there's no problem, don't annoy the user (or admin or whoever reads your log files) with unnecessary noise.

    Same situation: If you're looking for a file that can be (validly) in either of two locations, and you don't find it in location A, then find it in B, you don't log the "failure" to find it in A (even if this might be signalled via an exception, depending on how you do it), because finding it in B is alright. If you think otherwise, ask yourself: If you do find it in A, do you then check if it's also in B and log so if not (assuming A and B are equivalent)? And if you do find it in B as well, you should probably log this because one of them is probably redundant. You're now at the point where you log something every time.

    Professional doesn't mean blindly following rules, but also applying common sense. Sometimes (not often, though) it is OK to just ignore an exception.

  • QJo (unregistered) in reply to Zagyg
    Zagyg:
    Of course, since he was the apex predator, nobody would (could) review his code
    Really? Really??? I'm not going to "TRWTF is..." but wherever I've worked (big or small), the top fish have never been so arrogant as to not request a junior to sanity check (or "formally review") their code if there's no one above them.

    A proper apex predator would be so proud of his expertise that he would be expecting the juniors to line up outside his office door for the chance of receiving the benefit of his wisdom. He would be desperate to show off his brilliance.

  • tfwrt (unregistered)

    the real wtf is that all u nerds are gittin ur jimimes in a wads wehn its clearly a bs / story

  • (cs) in reply to foo AKA fooo
    foo AKA fooo:
    Spamming log files with things that are not errors is not professional but bureacratic.

    I'm just gonna stop you right there and say NO.

  • Not snoofle (unregistered)

    I am amazed how little I like snoofle's writing, even in articles like this. I only look at the byline when I'm disappointed and it's always him... The tone is always so much meaner than the old Alex stories.

  • xf (unregistered)

    what does "wonded" mean?

  • Yuri (unregistered) in reply to emaN ruoY
    emaN ruoY:
    Zagyg:
    Of course, since he was the apex predator, nobody would (could) review his code
    Really? Really??? I'm not going to "TRWTF is..." but wherever I've worked (big or small), the top fish have never been so arrogant as to not request a junior to sanity check (or "formally review") their code if there's no one above them.
    Welcome to The Daily WTF, where most stories involving code are either performed by juniors or seniors, were the experienced worker has no time or no influential power to review code that ends up in production. And a QA check may or may not catch a thread overrun. (If they even have a QA team/person.)
    Is there somethign between a Junior and a Senior? No, Seriously.....
  • sha (unregistered) in reply to Not snoofle
    Not snoofle:
    I am amazed how little I like snoofle's writing, even in articles like this. I only look at the byline when I'm disappointed and it's always him... The tone is always so much meaner than the old Alex stories.
    I agree - there's an air of 'Holier than thou' more noticeable than any of the other writers....
  • Dan the man (unregistered) in reply to chubertdev
    chubertdev:
    foo AKA fooo:
    ... which seems to be on the level of "smallowing exceptions = always bad". With a higher level of experience, you might realize someday that this isn't an absolute rule. In this case, the exception is that a sleep was interrupted; furthermore the sleep is used in a retry loop. Therefore, cutting the sleep short will do no more harm than retrying a little earlier. (This is not meant to suggest that the code is sane in any other way, of course, but if you have a valid case for a retry-sleep loop, ignoring short sleeps is usually not a problem. In fact, any effort to complete the sleep exactly is usually unnecessary complexity and therefore bad.)

    No, you at the very least log it. If you think otherwise, you have a very, very long way to go in your profession.

    And what do you expect your production team to do when they find the offending log entry in the logs? And how will you fix the code when you get a ticket raised by this 'problem'.

    The answer is that they can't do anything about it, and there is no way to improve this part of the code (except perhaps a more detailed comment), as fooo mentioned.

    But don't let experience and detailed knowledge get in the way of your idealistic blustering. That would take away all the fun for the rest of us.

  • foo AKA fooo (unregistered) in reply to Dan the man
    Dan the man:
    chubertdev:
    foo AKA fooo:
    ... which seems to be on the level of "smallowing exceptions = always bad". With a higher level of experience, you might realize someday that this isn't an absolute rule. In this case, the exception is that a sleep was interrupted; furthermore the sleep is used in a retry loop. Therefore, cutting the sleep short will do no more harm than retrying a little earlier. (This is not meant to suggest that the code is sane in any other way, of course, but if you have a valid case for a retry-sleep loop, ignoring short sleeps is usually not a problem. In fact, any effort to complete the sleep exactly is usually unnecessary complexity and therefore bad.)

    No, you at the very least log it. If you think otherwise, you have a very, very long way to go in your profession.

    And what do you expect your production team to do when they find the offending log entry in the logs? And how will you fix the code when you get a ticket raised by this 'problem'.

    The answer is that they can't do anything about it, and there is no way to improve this part of the code (except perhaps a more detailed comment), as fooo mentioned.

    But don't let experience and detailed knowledge get in the way of your idealistic blustering. That would take away all the fun for the rest of us.

    Well, he said no. In caps. And bolded. Can't argue against that. I bow to his professionalism.

  • Dan the man (unregistered) in reply to Anonymous Cowherd
    Anonymous Cowherd:

    ... and with a slightly higher level of experience, you might realize someday that interrupting a thread is the standard way of asking a thread to clean up after itself and shut down, and the InterruptedException is how that request is received in the thread. So yes, while it may be okay to ignore some exceptions (NumberFormatException comes to mind, if you're sure about your inputs), this isn't really one of those cases.

    So..... if the thread is being shut down, surely the server is being shut down? Why do you want to log the fact that it happened to be sleeping at the exact moment when the server was being shut down. What do you gain from that?

    Or are you imagining some sort of thread manager shutting down the thread because it has taken too long? In that case (with such a short wait time), I would prefer to have the logic inline in this loop, and avoid relying on an exception execution path.

    Either way, I'm genuinely interested whether anyone can actually improve on this code. As far as I can tell, the exception is just used as a sort of in-your-face documentation to remind the developer that a sleep operation can be interrupted. I can't remember ever reacting to an interruption in my code, but then again, I mostly do JEE stuff, and spawning threads is a no-no, and I would avoid sleeping at almost all costs.

  • A. Hitler (unregistered)

    There's more than one way to shut down a thread.

  • (cs) in reply to Dan the man
    Dan the man:
    chubertdev:
    foo AKA fooo:
    ... which seems to be on the level of "smallowing exceptions = always bad". With a higher level of experience, you might realize someday that this isn't an absolute rule. In this case, the exception is that a sleep was interrupted; furthermore the sleep is used in a retry loop. Therefore, cutting the sleep short will do no more harm than retrying a little earlier. (This is not meant to suggest that the code is sane in any other way, of course, but if you have a valid case for a retry-sleep loop, ignoring short sleeps is usually not a problem. In fact, any effort to complete the sleep exactly is usually unnecessary complexity and therefore bad.)

    No, you at the very least log it. If you think otherwise, you have a very, very long way to go in your profession.

    And what do you expect your production team to do when they find the offending log entry in the logs? And how will you fix the code when you get a ticket raised by this 'problem'.

    The answer is that they can't do anything about it, and there is no way to improve this part of the code (except perhaps a more detailed comment), as fooo mentioned.

    But don't let experience and detailed knowledge get in the way of your idealistic blustering. That would take away all the fun for the rest of us.

    I'm soooooooooooooo glad that I don't work with you.

  • (cs) in reply to foo AKA fooo
    foo AKA fooo:
    Well, he said no. In caps. And bolded. Can't argue against that. I bow to his professionalism.

    You obviously don't want to change your mind, so I won't waste my time. Ignorance is bliss, on multiple levels in this case.

  • ejtaw (unregistered) in reply to chubertdev
    chubertdev:
    Dan the man:
    chubertdev:
    foo AKA fooo:
    ... which seems to be on the level of "smallowing exceptions = always bad". With a higher level of experience, you might realize someday that this isn't an absolute rule. In this case, the exception is that a sleep was interrupted; furthermore the sleep is used in a retry loop. Therefore, cutting the sleep short will do no more harm than retrying a little earlier. (This is not meant to suggest that the code is sane in any other way, of course, but if you have a valid case for a retry-sleep loop, ignoring short sleeps is usually not a problem. In fact, any effort to complete the sleep exactly is usually unnecessary complexity and therefore bad.)

    No, you at the very least log it. If you think otherwise, you have a very, very long way to go in your profession.

    And what do you expect your production team to do when they find the offending log entry in the logs? And how will you fix the code when you get a ticket raised by this 'problem'.

    The answer is that they can't do anything about it, and there is no way to improve this part of the code (except perhaps a more detailed comment), as fooo mentioned.

    But don't let experience and detailed knowledge get in the way of your idealistic blustering. That would take away all the fun for the rest of us.

    I'm soooooooooooooo glad that I don't work with you.

    Threads go crazy without enough sleep....

    Obligatory xkcd.

    Also Akismet sucks dead goats' fallopian tubes

  • (cs) in reply to ejtaw
    ejtaw:
    Threads go crazy without enough sleep....

    Obligatory xkcd.

    I thought you were going to go for this one.

  • Meep (unregistered) in reply to threadbear
    threadbear:
    c) multithreaded systems are notoriously prone to these type of problems when developers armed with their newly acquired multithreading hammer start to see everything as a nail.

    Ugh refactoring threading...

    First you try to clean up the logging, because there are all these places they're catching every exception and tedious logging it. So you go back to the thread construction, centralize it, replace the catch(Throwable) with the actual exceptions, and add an uncaught thread death handler since there are invariably a mess of NPEs and such.

    Then you start looking at the flow, and naturally they're using the a rat's nest of flags and waits and notifies. You slowly untangle the mess and finally replace it all with a call to join.

    And naturally they're not writing data back in a queue, they used something stupid like a Vector, which they've further synchronized on whatever.

    And finally it dawns on you that they're not doing anything in the calling thread but waiting for the queue to fill, so the whole thread was completely unnecessary and can be replaced with a simple method call that populates an ArrayList.

  • Meep (unregistered) in reply to Dan the man
    Dan the man:
    Anonymous Cowherd:

    ... and with a slightly higher level of experience, you might realize someday that interrupting a thread is the standard way of asking a thread to clean up after itself and shut down, and the InterruptedException is how that request is received in the thread. So yes, while it may be okay to ignore some exceptions (NumberFormatException comes to mind, if you're sure about your inputs), this isn't really one of those cases.

    So..... if the thread is being shut down, surely the server is being shut down? Why do you want to log the fact that it happened to be sleeping at the exact moment when the server was being shut down. What do you gain from that?

    There is no reason to assume that a thread is only shut down when the entire process is terminating.

    It only means a certain task is being interrupted. For instance, if I'm doing a file transfer and want the user to be able to click "Cancel," I want to stop the transfer promptly. There's no reason to assume the entire application is shutting down.

    Similarly, Tomcat has the ability to load a new version of a web app in place. What it does is it loads the new version and starts handing new requests to the new version. When the old version has no more active requests, it is directed to shut down. (It's pretty slick when it works.)

    So a web app in Tomcat needs to release any threads its using in a timely fashion, but it explicitly does not mean that the entire process is terminating.

    In most cases, instead of swallowing the exception, you just return or throw some kind of unchecked exception. Or, if you can't handle the interrupt immediately, you use Thread.interrupt() to set the thread's interrupt flag and the next block that can throw an InterruptedException will throw immediately.

    Either way, I'm genuinely interested whether anyone can actually improve on this code.

    Really, you think it makes sense to hammer the database every 10 ms until the number of rows has actually increased?

    The commit method in JDBC won't return until the DBMS has actually committed the transaction. You can eliminate all of that and just run the "write message to DBMS" in a regular method call.

    If you're interested in better concurrency with a DBMS, you buy an expensive DBMS and tune it. Apps should just send the data in as big batches as they can and only open more connections if it's convenient.

Leave a comment on “Throttling Throughput”

Log In or post as a guest

Replying to comment #:

« Return to Article