Scriptoptimierung aber wie?

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
liberavia
User
Beiträge: 19
Registriert: Donnerstag 28. August 2008, 12:27

Scriptoptimierung aber wie?

Beitragvon liberavia » Mittwoch 1. Oktober 2008, 08:07

Hallo leute. Habe hier ein Python-Script welches periodisch in einer Datenbank nach jobs schaut
und entsprechende bash scripte startet oder Kommandos direkt absetzt. Das Script
funktioniert, allerdings führt es auf meinem PIII 900MHz als Testmaschine zu einer Systemauslastung von ca. 78% :shock:.

Vermutlich sind meine Aufrufe von os.system Schuld daran. Allerdings hab ich es bisher noch
nicht hinbekommen zu lernnen, wie man es richtig macht. Kann mir mal jemand an diesem Script
demonstrieren, wie man das elegant löst?

Vielen Dank

André

Code: Alles auswählen

#!/usr/bin/python
import os
import MySQLdb
import string
import time
conffile = open("/etc/cbwebgui/cbconnect.conf","r")
for line in conffile:
   confentry = line.split("=")
   if confentry[0] == "dbhost":
      dbhost = confentry[1]
   elif confentry[0] == "dbuser":
      dbuser = confentry[1]
   elif confentry[0] == "dbpasswd":
      dbpasswd = confentry[1]
   elif confentry[0] == "dbname":
      dbname = confentry[1]
conffile.close()
connection = MySQLdb.connect(dbhost[0:-1],dbuser[0:-1],dbpasswd[0:-1],dbname[0:-1])
cursor = connection.cursor()
while True:
   # get the next job
   cursor.execute("SELECT id, command, argument, timestamp FROM jobs_current ORDER BY timestamp LIMIT 1")
   res = cursor.fetchone()
   if res != None:
      # There is a job to do and here are the params
      job_id = res[0]
      job_command = res[1]
      job_argument = res[2]
      job_timestamp = res[3]
      # seperate commands
      if job_command == "restart":
         # seperate restart arguments
         if job_argument == "now":
            os.system("asterisk -rx \"restart now\"")
         elif job_argument == "gracefully":
            os.system("asterisk -rx \"restart gracefully\"")
      elif job_command == "reload":
         # seperate reload arguments
         if job_argument == "all":
            os.system("asterisk -rx reload")
         elif job_argument == "dialplan":
            os.system("asterisk -rx \"dialplan reload\"")
         elif job_argument == "sip":
            os.system("asterisk -rx \"sip reload\"")
      elif job_command == "write":
         # seperate write arguments execute the related script
         if job_argument == "extensions":
            os.system("wextensions %s %s %s" % (dbuser[0:-1],dbpasswd[0:-1],dbname[0:-1]))
         elif job_argument == "sip":
            os.system("wsip %s %s %s" % (dbuser[0:-1],dbpasswd[0:-1],dbname[0:-1]))
         elif job_argument == "queues":
            os.system("wqueues %s %s %s" % (dbuser[0:-1],dbpasswd[0:-1],dbname[0:-1]))
         elif job_argument == "voicemail":
            os.system("wvoicemail %s %s %s" % (dbuser[0:-1],dbpasswd[0:-1],dbname[0:-1]))
         elif job_argument == "agents":
            os.system("wagents %s %s %s" % (dbuser[0:-1],dbpasswd[0:-1],dbname[0:-1]))
         elif job_argument == "meetme":
            os.system("wmeetme %s %s %s" % (dbuser[0:-1],dbpasswd[0:-1],dbname[0:-1]))   
         elif job_argument == "manager":
            os.system("wmanager %s %s %s" % (dbuser[0:-1],dbpasswd[0:-1],dbname[0:-1]))   
      # delete last entry and enter values into jobs_done
      cursor.execute("DELETE FROM jobs_current WHERE id='%s'" % job_id)
      # cursor.execute("INSERT INTO `callbuntu`.`jobs_done` (id, job_id, timestamp, command, argument) VALUES (NULL, %s, %s, %s, %s)" % (job_id, job_timestamp, job_command, job_argument) )
      time.sleep(1)
Die Entscheidung zwischen OpenSource- und proprietärer Software ist die Entscheidung zwischen Evolution und Marketing.
Welches dieser Prinzipien ist Deiner Meinung nach bewährter und nachhaltiger?
Benutzeravatar
jens
Moderator
Beiträge: 8458
Registriert: Dienstag 10. August 2004, 09:40
Wohnort: duisburg
Kontaktdaten:

Re: Scriptoptimierung aber wie?

Beitragvon jens » Mittwoch 1. Oktober 2008, 08:44

Nur ein paar Anmerkungen:
In deinem Skript sind auffällig viele Teile fast gleich. An einigen Stellen kann man besser ein daten dict nutzten. Besonders bei der job command Geschichte.

Ich würde die DB Daten nicht in Variablen packen, sondern in ein dict:

Code: Alles auswählen

...
db_settings = {}
for line in conffile:
    kind, data = line.split("=",1)
    db_settings[kind] = data.strip()
...
connection = MySQLdb.connect(
    db_settings["dbhost"], db_settings["dbuser"],
    db_settings["dbpasswd"], db_settings["dbname"]
)
...


Das hier:

Code: Alles auswählen

        job_id = res[0]
        job_command = res[1]
        job_argument = res[2]
        job_timestamp = res[3]

müßte auch so gehen:

Code: Alles auswählen

job_id, job_command, job_argument, job_timestamp = res

Vielleicht noch einfacher, du nutzt den MySQL Dict-Cursor:

Code: Alles auswählen

cursor = connection.cursor(MySQLdb.cursors.DictCursor)


Noch was: Ich versuche immer verschachtelungen nicht zu tief werden zu lassen. In deinem Falle würde ich bei if res != None: mit continue arbeiten, also:

Code: Alles auswählen

if res == None:
    continue

Der Code danach ist dann nicht nochmal eingerückt. Noch besser wäre es wohl die ganze Sache ein wenig in Funktionen Aufzuteilen ;)

Statt os.system würde ich subprocess nutzten, siehe: [wiki]Neue Features#Subprocess[/wiki]

All das hat aber wohl nichts mit dem eigentlichen Problem zu tun.

CMS in Python: http://www.pylucid.org
GitHub | Open HUB | Xing | Linked in
Bitcoins to: 1JEgSQepxGjdprNedC9tXQWLpS424AL8cd
liberavia
User
Beiträge: 19
Registriert: Donnerstag 28. August 2008, 12:27

Beitragvon liberavia » Mittwoch 1. Oktober 2008, 08:58

Vielen Dank für die vielen Anregungen :D

All das hat aber wohl nichts mit dem eigentlichen Problem zu tun.


Was hast du denn im Verdacht, was die Systemauslastung so hoch treibt?
Die Entscheidung zwischen OpenSource- und proprietärer Software ist die Entscheidung zwischen Evolution und Marketing.

Welches dieser Prinzipien ist Deiner Meinung nach bewährter und nachhaltiger?
Benutzeravatar
jens
Moderator
Beiträge: 8458
Registriert: Dienstag 10. August 2004, 09:40
Wohnort: duisburg
Kontaktdaten:

Beitragvon jens » Mittwoch 1. Oktober 2008, 09:00

Ich denke die System Aufrufe. Messe doch mal die Zeit, während das Programm einen Job abarbeitet.
Du könntest auch mal time.sleep höher setzten und dann die CPU Belastung vergleichen.

CMS in Python: http://www.pylucid.org
GitHub | Open HUB | Xing | Linked in
Bitcoins to: 1JEgSQepxGjdprNedC9tXQWLpS424AL8cd
sma
User
Beiträge: 3018
Registriert: Montag 19. November 2007, 19:57
Wohnort: Kiel

Beitragvon sma » Mittwoch 1. Oktober 2008, 09:02

Dir ist schon klar, dass du FORTLAUFEND in die Datenbank schaust, wenn sie leer ist und nur dann 1 Sekunde wartest, wenn einen Job gefunden, abgearbeitet und gelöscht hast? Du willst bestimmt das sleep() in Zeile 64 einmal ausrücken, damit es immer ausgeführt wird.

Übrigens, den Ansatz ist nicht threadsafe - solltest du jemals zwei deiner Prozesse starten wollen wird es unweigerlich zu Konflikten kommen und zwei Prozesse werden den selben Job ausführen wollen.

Stefan
liberavia
User
Beiträge: 19
Registriert: Donnerstag 28. August 2008, 12:27

Beitragvon liberavia » Mittwoch 1. Oktober 2008, 09:15

Du willst bestimmt das sleep() in Zeile 64 einmal ausrücken, damit es immer ausgeführt wird.


Hoppla :oops: Jetzt weiß ich auch warum die erhöhung des Wertes keine Auswirkung hatte ;)

solltest du jemals zwei deiner Prozesse starten wollen...


Nein, hatte ich nicht vor. Jeder Job soll wie ein stack einzeln abgearbeitet werden.

Aber gegen einen besseren Ansatz habe ich natürlich nix einzuwenden.

Werde die Ansätze so gegen Mittag versuchen umzusetzen, da ich grad noch bei der Weboberfläche bin und diese gegen Falscheingaben absichere...

Werde dann berichten.

Bis denne

André
Die Entscheidung zwischen OpenSource- und proprietärer Software ist die Entscheidung zwischen Evolution und Marketing.

Welches dieser Prinzipien ist Deiner Meinung nach bewährter und nachhaltiger?
Benutzeravatar
jens
Moderator
Beiträge: 8458
Registriert: Dienstag 10. August 2004, 09:40
Wohnort: duisburg
Kontaktdaten:

Beitragvon jens » Mittwoch 1. Oktober 2008, 09:17

liberavia hat geschrieben:da ich grad noch bei der Weboberfläche bin und diese gegen Falscheingaben absichere...

forms kann da weiterhelfen: http://docs.djangoproject.com/en/dev/topics/forms/

CMS in Python: http://www.pylucid.org
GitHub | Open HUB | Xing | Linked in
Bitcoins to: 1JEgSQepxGjdprNedC9tXQWLpS424AL8cd
liberavia
User
Beiträge: 19
Registriert: Donnerstag 28. August 2008, 12:27

Beitragvon liberavia » Mittwoch 1. Oktober 2008, 09:38

Die Weboberfläche habe ich mit PHP geschrieben ;)

Trotzdem Danke
Die Entscheidung zwischen OpenSource- und proprietärer Software ist die Entscheidung zwischen Evolution und Marketing.

Welches dieser Prinzipien ist Deiner Meinung nach bewährter und nachhaltiger?
sma
User
Beiträge: 3018
Registriert: Montag 19. November 2007, 19:57
Wohnort: Kiel

Beitragvon sma » Mittwoch 1. Oktober 2008, 09:56

Um mehr als einen Worker-Prozess zu erlauben, könnte folgendes auch ohne Transaktionen funktionieren:

Füge eine weitere Spalte PID in die Tabelle ein. Diese enthält normalerweise NULL. Bestimme die eigene Prozess-ID. Statt eines SELECT mache jetzt ein "UPDATE jobs_current SET pid=? WHERE pid IS NULL ORDER BY timestamp LIMIT 1" mit der eigenen ID als Argument. Dieses sperrt atomar einen Datensatz gegenüber allen anderen Prozessen. Lade danach den Datensatz mit "SELECT ... WHERE pid=?" falls das Update erfolgreich war. Lösche nach Abarbeitung des Jobs normal den Datensatz.

Nachteil ist, dass so ein Job hängen bleiben kann, wenn dein Worker vor dem Löschen aber nach dem Sperren abstürzt. Das könnten Transaktionen verhindern. Ohne Transaktionen musst du manuell entsperren, etwa indem du periodisch prüfst, ob zu den PIDs in der Datenbank überhaupt Unix-Prozesse existieren. Ein stat() auf /proc/%d sollte reichen, denn so schnell werden die Nummern meines Wissens nicht wiedervergeben.

Stefan

PS: Ich würde das ganze etwas generischer gestalten. Baue dir eine Abbildung von job_command auf das Unix-Kommando und benutzte dort nicht %s sondern %(xyz)s, damit du dann ein dict mit allen möglichen Konfigurationsparametern übergeben kannst und nicht bei jedem Befehl wissen musst, was er wohl braucht.

Code: Alles auswählen

commands = {
  'manager': 'wmanager %(user)s %(passwd)s %(db)s',
  ...
}
config = {
  'user': ...,
  ...
}
def execute(command):
  return os.system(commands[command] % config) == 0 # kann 2x KeyError werfen: Abfangen!
BlackJack

Beitragvon BlackJack » Mittwoch 1. Oktober 2008, 10:10

@liberavia: Ein wenig Kritik: Zu viel Code auf Modulebene, da sollten möglichst nur Definitionen stehen und das Hauptprogramm sollte in einer Funktion verschwinden.

Für die Konfigurationsdatei würde ich den `ConfigParser` aus der Standardbibliothek verwenden statt selber etwas zu basteln.

So wie's jetzt gelöst ist, wird auch gar nicht überprüft, ob auch alle Optionen gesetzt sind. Und der Code fällt auf die Nase wenn jemand zum Beispiel in seinem Passwort ein '=' verwendet.

Das entfernen des letzten Zeichens machst Du an der falschen Stelle, diese Operation sollte man *einmal* machen. Insbesondere bei `dbname` fällt das auf.

Die einzelnen Jobs würde ich in eigene Funktionen stecken statt diesem riesigen ``if``/``elif``-Gebilde.

`os.system()` ist IMHO keine gute Idee, weil Du Dich um das "escapen" der Argumente für die Shell selber kümmern musst, bzw. da Du das gar nicht tust Probleme mit Aufrufen bis hin zu Sicherheitslücken hast, wenn jemand über die Datenbank "bösartige" Werte an die Shell übergeben lässt. Wenn es da eine Lücke gibt, können beliebige Programme mit den Rechten dieses Skriptes ausgeführt werden.

Ich sehe kein `commit()` bei der Datenbank. Die Zeiten, dass man bei MySQL grundsätzlich sagen konnte, dass man das nicht braucht, sind ja nun auch schon eine Weile vorbei. Und auch wenn das bei der ID wahrscheinlich unproblematisch ist, sollte man aus Sicherheitsgründen keine Werte per Zeichenkettenformatierung in SQL-Anweisungen bringen, sondern das zweite Argument von `execute()` verwenden.

Und natürlich die üblichen Stilkommentare: Zeilen die länger als 80 Zeichen sind. Keine Leerzeichen hinter Kommata.
lunar

Beitragvon lunar » Mittwoch 1. Oktober 2008, 10:35

stat() auf /proc/%d setzt die Existenz des /proc-Verzeichnisses vorraus, was nicht bei jedem Unix der Fall ist. Auch ist diese Operation nicht atomar, und deswegen eine potentielle Race Condition.

Gegen die Benutzung von os.system() sprechen neben potentiellen Sicherheitslücken noch die Schwierigkeiten bei der Fehlerbehandlung. Um Fehler wie nicht existierende Programmdateien zu erkennen, ist man auf die nicht standardisierten Rückgabewerte der Shell und der Funktion selbst angewiesen. subprocess dagegen wirft in solchen Fällen sinnvolle Ausnahmen.
BlackJack

Beitragvon BlackJack » Mittwoch 1. Oktober 2008, 10:52

Ich hab's mal überarbeitet: http://paste.pocoo.org/show/86694/

Ist natürlich ungetestet.
sma
User
Beiträge: 3018
Registriert: Montag 19. November 2007, 19:57
Wohnort: Kiel

Beitragvon sma » Mittwoch 1. Oktober 2008, 11:04

Irgendwie hatte ich Linux angenommen. Doch auch ohne /proc kommt man sicherlich irgendwie an die IDs der laufenden Prozesse heran. Zur Not ruft man "ps" auf - ob nun per os.system oder anders ist mir egal. Mir ging es bei meinem Beitrag nur darum, sicher mit mehreren Worker-Prozessen arbeiten zu können - ohne das man Datenbank-Transaktionen braucht.

In wie fern ist da der Test, ob ein Worker noch lebt, eine Racing-Condition? Habe ich da einen Denkfehler?

Ich bestimme die Liste der (ID, PID) aus der Datenbank wo PID != NULL. Dies sind die aktiven bzw. hängen gebliebenden Jobs. Nun schaue ich, gruppiert nach PID, ob der Prozess noch lebt. Falls nein, ist das ein hängender Job und ich wiederbelebe alle diese Jobs, indem ich PID auf NULL setze. Was ich bei diesem Durchlauf nicht erwische - etwa weil ein Prozess gerade abnibbelt - das kommt beim nächsten Durchlauf in wenigen Sekunden oder Minuten dran. Was mir nicht passieren darf ist, dass eine PID vom OS wiederverwendet wird, sodass ich einen eigentlich toten Job für lebendig halte.

Stefan
Y0Gi
User
Beiträge: 1454
Registriert: Freitag 22. September 2006, 23:05
Wohnort: ja

Beitragvon Y0Gi » Mittwoch 1. Oktober 2008, 11:24

Auf `None` sollte man mit `is` bzw. `is not` testen, nicht mit `==`/`!=`, hab ich mal irgendwo gelesen. Und nein, ich denke nicht, dass das hier den Performance-Ausschlag geben wird ;)
Benutzeravatar
jens
Moderator
Beiträge: 8458
Registriert: Dienstag 10. August 2004, 09:40
Wohnort: duisburg
Kontaktdaten:

Beitragvon jens » Mittwoch 1. Oktober 2008, 11:34


CMS in Python: http://www.pylucid.org
GitHub | Open HUB | Xing | Linked in
Bitcoins to: 1JEgSQepxGjdprNedC9tXQWLpS424AL8cd

Wer ist online?

Mitglieder in diesem Forum: brainstir