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.

Sign up for GitHub

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

Jump to bottom

Normalize thread naming of threads in add-ons #8216

Open
Hilbrand opened this issue Jul 28, 2020 · 6 comments · May be fixed by #17804
Open

Normalize thread naming of threads in add-ons #8216

Hilbrand opened this issue Jul 28, 2020 · 6 comments · May be fixed by #17804
Labels
help wanted

Comments

@Hilbrand
Copy link
Member

Hilbrand commented Jul 28, 2020 •
edited
Loading

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
@lolodomo
[rotel] Thread naming
4b8bbdf 
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
@lolodomo lolodomo mentioned this issue Aug 1, 2020
Merged
fwolter pushed a commit that referenced this issue Aug 1, 2020
@lolodomo
[rotel] Thread naming (#8234)
81722d4 
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
@lolodomo
[rfxcom] Thread naming
c1ea97f 
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
@lolodomo lolodomo mentioned this issue Aug 1, 2020
Merged
fwolter pushed a commit that referenced this issue Aug 1, 2020
@lolodomo
[rfxcom] Thread naming (#8235)
b36c6b5 
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
@lolodomo
[powermax] Thread naming
12a00cd 
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
@lolodomo lolodomo mentioned this issue Aug 1, 2020
Merged
lolodomo added a commit to lolodomo/openhab-addons that referenced this issue Aug 1, 2020
@lolodomo
[rfxcom] Thread name format updated
50c3548 
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
@lolodomo lolodomo mentioned this issue Aug 1, 2020
Merged
lolodomo added a commit to lolodomo/openhab-addons that referenced this issue Aug 1, 2020
@lolodomo
[rotel] Thread name format updated
c0880c8 
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
@lolodomo lolodomo mentioned this issue Aug 1, 2020
Merged
lolodomo added a commit to lolodomo/openhab-addons that referenced this issue Aug 1, 2020
@lolodomo
[rotel] Thread name format updated
0f50d7c 
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
martinvw pushed a commit that referenced this issue Aug 1, 2020
@lolodomo
[rfxcom] Thread name format updated (#8237)
cf65e5a 
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
@lolodomo
[rotel] Thread name format updated (#8238)
bd4d96a 
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
@lolodomo @MPH80
[rotel] Thread naming (openhab#8234)
6ce7e13 
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
@lolodomo @MPH80
[rfxcom] Thread naming (openhab#8235)
08a1583 
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
@lolodomo @MPH80
[rfxcom] Thread name format updated (openhab#8237)
83e2ff8 
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
@lolodomo @MPH80
[rotel] Thread name format updated (openhab#8238)
dc27a0c 
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
@lolodomo
[powermax] Thread naming (#8236)
d25cb4b 
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
@lolodomo @bern77
[rfxcom] Thread name format updated (openhab#8237)
120c059 
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
@lolodomo @bern77
[rotel] Thread name format updated (openhab#8238)
a9fe3d9 
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Bernhard <[email protected]>
@martinvw martinvw mentioned this issue Aug 12, 2020
Merged
@Hilbrand Hilbrand mentioned this issue Aug 21, 2020
Merged
@lolodomo lolodomo mentioned this issue Aug 26, 2020
Merged
DaanMeijer pushed a commit to DaanMeijer/openhab-addons that referenced this issue Sep 1, 2020
@lolodomo @DaanMeijer
[powermax] Thread naming (openhab#8236)
9a1fd24 
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Daan Meijer <[email protected]>
@fwolter fwolter mentioned this issue Sep 6, 2020
Merged
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this issue Sep 12, 2020
@lolodomo @CSchlipp
[rotel] Thread naming (openhab#8234)
c378f82 
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
@lolodomo @CSchlipp
[rfxcom] Thread naming (openhab#8235)
1933a4c 
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
@lolodomo @CSchlipp
[rfxcom] Thread name format updated (openhab#8237)
aa339bd 
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
@lolodomo @CSchlipp
[rotel] Thread name format updated (openhab#8238)
3471684 
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
@lolodomo @CSchlipp
[powermax] Thread naming (openhab#8236)
8be106c 
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
@lolodomo @markus7017
[rotel] Thread naming (openhab#8234)
c4accfd 
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
@lolodomo @markus7017
[rfxcom] Thread naming (openhab#8235)
61c7a8f 
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
@lolodomo @markus7017
[rfxcom] Thread name format updated (openhab#8237)
85e4292 
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
@lolodomo @markus7017
[rotel] Thread name format updated (openhab#8238)
8f39a21 
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
@lolodomo @markus7017
[powermax] Thread naming (openhab#8236)
dfb73ec 
Related to openhab#8216

Signed-off-by: Laurent Garnier <[email protected]>
@fwolter fwolter mentioned this issue Oct 23, 2020
Merged
@fwolter fwolter mentioned this issue Nov 1, 2020
Closed
@fwolter fwolter mentioned this issue Nov 24, 2020
Merged
@Hilbrand Hilbrand mentioned this issue Dec 25, 2020
Merged
Hilbrand added a commit to Hilbrand/openhab-addons that referenced this issue Dec 29, 2020
@Hilbrand
Normalized thread names
edf5694 
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
@Hilbrand
Normalized thread names
fc6357d 
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
@Hilbrand Hilbrand mentioned this issue Dec 29, 2020
Merged
Hilbrand added a commit to Hilbrand/openhab-addons that referenced this issue Jan 6, 2021
@Hilbrand
Normalized thread names
a4c91f0 
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
kaikreuzer pushed a commit that referenced this issue Jan 12, 2021
@Hilbrand
Normalized thread names (#9581)
ac3f907 
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
@Hilbrand @themillhousegroup
Normalized thread names (openhab#9581)
ed018d4 
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
Signed-off-by: John Marshall <[email protected]>
@lolodomo lolodomo mentioned this issue Aug 27, 2021
Merged
@fwolter fwolter mentioned this issue Sep 12, 2021
Merged
thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this issue Nov 7, 2021
@Hilbrand @thinkingstone
Normalized thread names (openhab#9581)
0705f92 
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
@Hilbrand Hilbrand mentioned this issue Feb 9, 2022
Merged
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this issue May 5, 2022
@Hilbrand
Normalized thread names (openhab#9581)
29b7540 
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?

@lsiepel lsiepel linked a pull request Nov 25, 2024 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Normalize thread names lsiepel/openhab-addons
4 participants
@Hilbrand @fwolter @kaikreuzer @lsiepel