0

I am trying to get rows for a particular machine name from v$session and spawning a child thread which calls a function called kill_it() to work on its particular row. And the main do_this() sleeps for 100sec and checks for newer rows.

def do_this():

    sql='select * from v$session where machine like abc'
    session=Popen(['sqlplus','-S','/ as sysdba'], stdin=PIPE, stdout=PIPE, stderr=PIPE)
    session.stdin.write(sql)
    stdout,stderr=session.communicate()
    stdout=stdout.strip()
    stdout=stdout.split('\n')

    for i in range (len(stdout)):

        if 'no rows selected' not in stdout:
            sql_output=stdout[i].split(',')
            client_info=sql_output[0]
            sid=sql_output[1]
            serial=sql_output[2]
            program=sql_output[3]
            last_call=sql_output[4]
            process=sql_output[5]
            machine=sql_output[6]

            t = Thread(target=kill_it, args=(client_info,sid,serial,program,last_call,process,machine,))
            print t
            t.start()



while True:
    do_this()
    time.sleep(100)

But in the kill_it() function which should run in its own thread, but when I am trying to put the child thread to sleep for 10secs, it is instead sleeping for 100secs or even if I remove the sleep it doesnt really keep looking for xyz in client info.

def kill_it(client_info,sid,serial,program,last_call,process,machine):

    while True:
        print "thread is :"+str(current_thread())
        last_call=int(last_call)
        if 'xyz' in client_info and last_call>100 :
            command='alter system kill session \''+sid+','+serial+'\' immediate;'
            print command
            '''
            session=Popen(['sqlplus','-S','/ as sysdba'], stdin=PIPE, stdout=PIPE, stderr=PIPE)
            session.stdin.write(command)
            stdout,stderr=session.communicate()
            print stdout
            print stderr
            '''

            print 'done'
            break;

        else:
            print 'Sleeping coz job didn`t run for more than 5mins'
            time.sleep(10)

WOrks as expected when there is one row but for more than one gives issues.

RamPrasadBismil
  • 579
  • 2
  • 10
  • 30
  • WORKED FINE WHEN I USED MULTIPROCESSING INSTEAD OF MULTITHREADING CAN ANYONE TELL ME WHAT WAS OVERLAPPING OR WHY EXACTLY MULTITHREADING DIDN`T WORK BUT MULTIPROCESSING DID – RamPrasadBismil Aug 07 '14 at 22:05
  • 1
    HOLY CAPS LOCK BATMAN – skrrgwasme Aug 07 '14 at 22:06
  • What output are you seeing? – skrrgwasme Aug 07 '14 at 22:08
  • Keep in mind that thanks to the [GIL](https://wiki.python.org/moin/GlobalInterpreterLock), multithreading in Python isn't truly working in parallel. If the same code was working fine with the multiprocessing library, you may have had a subtle bug (maybe infinite loop in kill_it()?) that you got away with when your main process could kill the subprocesses, but is now revealing itself when the main thread is blocked. Also, you should be joining all of your threads and subprocesses to make sure they're cleaned up. – skrrgwasme Aug 07 '14 at 22:12
  • But if I include process.join() it only runs one process at a time. – RamPrasadBismil Aug 07 '14 at 22:15
  • stackoverflow.com/questions/5210866/… - I am trying to remove defunct processes using this solution but its not particularly well explained. – RamPrasadBismil Aug 07 '14 at 23:49
  • If you are trying to implement some technique from another question that you can't get to work, you can always open your own new question. To avoid getting it marked as a duplicate, explain that you're trying to implement an answer to another question, provide a link, then clearly explain what isn't working (error messages, bad results, computer explodes, etc) and why your question is different. – skrrgwasme Aug 07 '14 at 23:55
  • IMHO, the approach outlined in the other question is probably not the ideal approach. In fact, I think I'm going to leave a new answer on that question tomorrow. Python has too many cool multiprocessing options at your disposal for adding concurrency to be a good solution to concurrency management issues. Check back on that question later tomorrow and you'll have a new suggestion you can try out. – skrrgwasme Aug 08 '14 at 00:05
  • If you're interested, I just posted my answer on that other question [here](http://stackoverflow.com/a/25194652/2615940). @user2601010 – skrrgwasme Aug 08 '14 at 01:12

1 Answers1

1

Firstly:

In regards to your comments, you MUST ALWAYS join threads and processes that you spawn to ensure everything is cleaned up. If that interrupts your program's flow in an intrusive way, then you need to redesign your program because something is wrong. You should take a closer look at the Python multiprocessing library, in particular, look at multiprocessing.Pool().

Secondly:

Take a close look at your do_this() function:

# stuff...
stdout=stdout.strip()
stdout=stdout.split('\n')

You can daisy-chain string operations:

# stuff..
stdout=stdout.strip().split('\n')

Now on to your loop. I don't think it's doing what you really intended, and has several things in it you can improve:

# the variable 'stdout' is now a tuple as returned by string.split('\n'), right?

# if you need a loop index, use enumerate() instead of this
for i in range (len(stdout)):

    # this if statement checks for membership in the tuple 'stdout' which is 
    # going to have the same result every time - did you mean to check the
    # elements of the tuple you're iterating over for something instead?
    if 'no rows selected' not in stdout: 

        # indexing into an iterable with a loop index is usually wrong in python 
        # not always, but usually
        sql_output=stdout[i].split(',') 

        # do you need to split this thing up into so many components when they're
        # all getting passed anyway?
        client_info=sql_output[0] 
        sid=sql_output[1]
        serial=sql_output[2]
        program=sql_output[3]
        last_call=sql_output[4]
        process=sql_output[5]
        machine=sql_output[6]

        t = Thread(target=kill_it, args=(client_info,sid,serial,program,last_call,process,machine,))
        print t
        t.start()
        # threads and processes should ALWAYS be joined

I would suggest refactoring your do_this() code to something like this:

def do_this():

    sql='select * from v$session where machine like abc'
    session=Popen(['sqlplus','-S','/ as sysdba'], stdin=PIPE, stdout=PIPE, stderr=PIPE)
    session.stdin.write(sql)
    stdout,stderr=session.communicate()
    stdout=stdout.strip().split('\n')

    if 'no rows selected' not in stdout:
        pool = multiprocessing.Pool()
        pool.map(kill_it, stdout) # blocking call until all are done
        pool.close()
        pool.join()

    return

pool.map() calls the function kill_it on every object in the iterable passed to it, which in this case is the tuple that resulted from splitting stdout. If you choose to do this, it will be up to you to move the job of splitting the line on the commas into the kill_it function and replace the long argument list with a single argument: the string to be split.

TL;DR

I see two issues that could be causing you problems:
1. ALWAYS join your threads and processes. You can get odd behavior if you don't.
2. Check your if statement in the for loop of do_this(). I don't think you really meant to check the entire stdout tuple for 'no rows selected' there, because it is going to be the same result every time, so it should be in a loop. You may be killing/not killing rows you did/did not intend to.

skrrgwasme
  • 9,358
  • 11
  • 54
  • 84