Uploaded image for project: 'sfc'
  1. sfc
  2. SFC-164

The OpendaylightSfc singleton should be refactored to work better with the new blueprint changes

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Done
    • None
    • unspecified
    • General
    • None
    • Operating System: All
      Platform: All

    • 6768

      Suggestions for improvements by Alexis de Talhouƫt from this code review:

      https://git.opendaylight.org/gerrit/#/c/45438/1/sfc-ovs/src/main/resources/org/opendaylight/blueprint/sfc-ovs.xml

      By looking closer at it, it seems this class is providing those things:

      InstanceIdentifier across the project
      ExecutorService statically defined
      close method clearing the whole SFC datastore
      an init method called in the sfc-provider
      some utility getters providing things given by blueprint's service registery

      I think the main concerns of having this class new in every bundle are the close() method and the ExecutorService definition (thread pool of 100). Regarding the close() method, it is well-know that ODL doesn't support uninstall (yet), thus not such a big deal, but it can blow here and there on shutdown because of this. Regarding the ExecutorService, this is more concerning, because you don't want to exhaust the thread pool available in the host running ODL (even though this might not happen, you better be safe). Moreover, the initial intention was to have only one thread pool of 100 threads shared across the whole SFC project.

      To sum up, I guess for now this is fine, but once all blueprint patches are there, the OpendaylightSfc class needs to be rework and/or break down, and the ExecturoService needs to become shared across all bundle. To do the later, you can defined a FixedThreadPoolWrapper ( https://github.com/opendaylight/controller/blob/master/opendaylight/config/threadpool-config-impl/src/main/java/org/opendaylight/controller/config/threadpool/util/FixedThreadPoolWrapper.java ) in the sfc-provider, and expose the ThreadPool service so it can be gathered from the service registery.

            guillermo.pablo.tomasini.redondas@ericsson.com Guillermo Pablo Tomasini
            ebrjohn Brady Johnson
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated:
              Resolved: