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

Normalize thread naming of threads in add-ons #8216

Closed
Hilbrand opened this issue Jul 28, 2020 · 6 comments · Fixed by #17804
Closed

Normalize thread naming of threads in add-ons #8216

Hilbrand opened this issue Jul 28, 2020 · 6 comments · Fixed by #17804

Comments

@Hilbrand
Copy link
Member

Hilbrand commented Jul 28, 2020

A number of add-ons start there own threads. To track threads better I propose these threads should all name their thread using a specific name convention. My proposal is to use:

  • If there is 1 thread::OH-binding-<thingUID>,
  • If there are multiple threads: OH-binding-<thingUID>-<custom>. Where custom is a developer defined word that suggest which specific thread it is.

Some bindings use Executors. My suggested would be to either standardize to all use the org.eclipse.smarthome.core.common. ThreadPoolManager or another option to allow a generic Executors, but initialize with the openHAB NamedThreadFactory. The named thread factory/pool manager should than be initialized with binding-<thingUID>. Note that using the ThreadPoolManager` will add an incremental number to the thread name for each thread started.

@fwolter
Copy link
Member

fwolter commented Jul 28, 2020

Many bindings append hostname and port number to the thread name. I think this is more meaningful than a number. I suggest to leave the part after the binding ID up to the binding developer.

@Hilbrand
Copy link
Member Author

Yes that might be a better idea, to leave it to the developer. Maybe the ThingID and some additional info so one can see which thing has started the thread. Or just instead of bindingId use the thing UID. I'll update the text.

@kaikreuzer
Copy link
Member

I'd like to mention that the general advice is to NOT use any threads at all within bindings. Most things should be possible to solve with the existing scheduler that is provided within the handlers. Requiring a dedicated thread should thus be an exception for the situations where it is really needed, such as continuously listening on TCP ports for events.

Please keep that in mind for all binding reviews!

lolodomo added a commit to lolodomo/openhab-addons that referenced this issue Aug 1, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
fwolter pushed a commit that referenced this issue Aug 1, 2020
Related to #8216

Signed-off-by: Laurent Garnier <[email protected]>
lolodomo added a commit to lolodomo/openhab-addons that referenced this issue Aug 1, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
fwolter pushed a commit that referenced this issue Aug 1, 2020
Related to #8216

Signed-off-by: Laurent Garnier <[email protected]>
@Hilbrand
Copy link
Member Author

Hilbrand commented Aug 1, 2020

@kaikreuzer my idea is to go through all bindings and see how threads are used and see if we can make them consistent. For example some use thread.start, while others use a single thread executor. Some use our thread executor to create a thread pool, others use the generic executor. I don't know what is preferred. But would like if we can make a general guideline. (Besides don't use a thead when not needed) Do you have an opinion about this?

lolodomo added a commit to lolodomo/openhab-addons that referenced this issue Aug 1, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
lolodomo added a commit to lolodomo/openhab-addons that referenced this issue Aug 1, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
lolodomo added a commit to lolodomo/openhab-addons that referenced this issue Aug 1, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
lolodomo added a commit to lolodomo/openhab-addons that referenced this issue Aug 1, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
martinvw pushed a commit that referenced this issue Aug 1, 2020
Related to #8216

Signed-off-by: Laurent Garnier <[email protected]>
@kaikreuzer
Copy link
Member

@Hilbrand Yes, I think a general guidelines makes sense.
Just like we offer thread pools through ThreadPoolManager, we should have a way to deal with single/permanent threads.
I don't have a strong opinion, how this guideline should look like, so feel free to suggest one. Aligning thread naming across all binings as a first step definitely makes sense as well 👍 .

Hilbrand pushed a commit that referenced this issue Aug 2, 2020
Related to #8216

Signed-off-by: Laurent Garnier <[email protected]>
MPH80 pushed a commit to MPH80/openhab-addons that referenced this issue Aug 3, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: MPH80 <[email protected]>
MPH80 pushed a commit to MPH80/openhab-addons that referenced this issue Aug 3, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: MPH80 <[email protected]>
MPH80 pushed a commit to MPH80/openhab-addons that referenced this issue Aug 3, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: MPH80 <[email protected]>
MPH80 pushed a commit to MPH80/openhab-addons that referenced this issue Aug 3, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: MPH80 <[email protected]>
fwolter pushed a commit that referenced this issue Aug 4, 2020
Related to #8216

Signed-off-by: Laurent Garnier <[email protected]>
bern77 pushed a commit to bern77/openhab-addons that referenced this issue Aug 6, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Bernhard <[email protected]>
bern77 pushed a commit to bern77/openhab-addons that referenced this issue Aug 6, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Bernhard <[email protected]>
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this issue Sep 12, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this issue Sep 12, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this issue Sep 12, 2020
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this issue Sep 12, 2020
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this issue Sep 12, 2020
markus7017 pushed a commit to markus7017/openhab-addons that referenced this issue Sep 19, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this issue Sep 19, 2020
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this issue Sep 19, 2020
markus7017 pushed a commit to markus7017/openhab-addons that referenced this issue Sep 19, 2020
markus7017 pushed a commit to markus7017/openhab-addons that referenced this issue Sep 19, 2020
Hilbrand added a commit to Hilbrand/openhab-addons that referenced this issue Dec 29, 2020
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
Hilbrand added a commit to Hilbrand/openhab-addons that referenced this issue Dec 29, 2020
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
Hilbrand added a commit to Hilbrand/openhab-addons that referenced this issue Jan 6, 2021
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
kaikreuzer pushed a commit that referenced this issue Jan 12, 2021
Related to #8216

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
themillhousegroup pushed a commit to themillhousegroup/openhab2-addons that referenced this issue May 10, 2021
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
Signed-off-by: John Marshall <[email protected]>
thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this issue Nov 7, 2021
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this issue May 5, 2022
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
@lsiepel
Copy link
Contributor

lsiepel commented Nov 25, 2024

How can we determine if all bindings have the thread name set correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants