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

Solucionamos escritura/lectura concurrente en graph #24

Merged
merged 1 commit into from
Jun 10, 2021

Conversation

adrianlzt
Copy link

Al hacer el último refactor para separar el procesamiento de métricas en
una función a parte se introdujo un error, no bloquear el grafo cuando
solicitamos los nodos child (Software) de un Server (LookupChildren en
la función processMetric).

Si en el momento de esa lectura, otra parte de skydive estaba intentando
escribir en el grafo, se producía un panic:
fatal error: concurrent map read and map write

Para solventar este problema se ha hecho una recolocación de los locks,
simplificando esta gestión.

Dentro de proccon los locks ahora quedan de la siguiente manera:

ServeHTTP
  processMetrics
    for hostMetrics
      LOCK
        get/create server
        processMetrics
        removeFromOthers
      UNLOCK
  generateOthers
    for hosts
      LOCK
      UNLOCK

cleanSoftwareNodes
  LOCK
  UNLOCK

De esta manera, cuando el servidor procesa un paquete de métricas, se
hace un lock por cada host que se está procesando y se elimina al
terminar ese host entero.

También solucionamos otra condición de carrera que se podía producir en
processMetrics, si entre que obteníamos un Server y se actualizaba su
metadata se producía alguna modificación en el nodo.
Esto era muy poco probable, ya que cada nodo en principio envía sus
métricas separadas por grandes intervalos de tiempo.

Al hacer el último refactor para separar el procesamiento de métricas en
una función a parte se introdujo un error, no bloquear el grafo cuando
solicitamos los nodos child (Software) de un Server (LookupChildren en
la función processMetric).

Si en el momento de esa lectura, otra parte de skydive estaba intentando
escribir en el grafo, se producía un panic:
fatal error: concurrent map read and map write

Para solventar este problema se ha hecho una recolocación de los locks,
simplificando esta gestión.

Dentro de proccon los locks ahora quedan de la siguiente manera:

ServeHTTP
  processMetrics
    for hostMetrics
    ¦ LOCK
    ¦   get/create server
    ¦   processMetrics
    ¦   removeFromOthers
    ¦ UNLOCK
  generateOthers
    for hosts
    ¦ LOCK
    ¦ UNLOCK

cleanSoftwareNodes
  LOCK
  UNLOCK

De esta manera, cuando el servidor procesa un paquete de métricas, se
hace un lock por cada host que se está procesando y se elimina al
terminar ese host entero.

También solucionamos otra condición de carrera que se podía producir en
processMetrics, si entre que obteníamos un Server y se actualizaba su
metadata se producía alguna modificación en el nodo.
Esto era muy poco probable, ya que cada nodo en principio envía sus
métricas separadas por grandes intervalos de tiempo.
@pablombg pablombg merged commit b242ead into datadope Jun 10, 2021
@pablombg pablombg deleted the bug/concurrent_map_read_write branch June 10, 2021 11:05
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

Successfully merging this pull request may close these issues.

None yet

2 participants