Skip to content

Make pstm handling thread sleep time configurable - #267

Merged
pnoltes merged 1 commit into
apache:masterfrom
Oipo:feature/configurable_sleeptime_pstm
Jul 13, 2020
Merged

Make pstm handling thread sleep time configurable#267
pnoltes merged 1 commit into
apache:masterfrom
Oipo:feature/configurable_sleeptime_pstm

Conversation

@Oipo

@Oipo Oipo commented Jul 9, 2020

Copy link
Copy Markdown
Contributor

No description provided.

add_test(NAME pstm_deadlock_udpmc_test COMMAND pstm_deadlock_udpmc_test WORKING_DIRECTORY $<TARGET_PROPERTY:pstm_deadlock_udpmc_test,CONTAINER_LOC>)
setup_target_for_coverage(pstm_deadlock_udpmc_test SCAN_DIR ..)

#TCP Endpoint test is disabled because the test is not stable when running on Travis

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this removed intentionally?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. If you look closely, it's trying to add a TCP test inside of the UDP block. The TCP block also adds this test, it must've somehow got duplicated.


manager->loghelper = logHelper;
manager->verbose = celix_bundleContext_getPropertyAsBool(context, PUBSUB_TOPOLOGY_MANAGER_VERBOSE_KEY, PUBSUB_TOPOLOGY_MANAGER_DEFAULT_VERBOSE);
manager->handlingThreadSleepTime = celix_bundleContext_getPropertyAsLong(context, PUBSUB_TOPOLOGY_MANAGER_HANDLING_THREAD_SLEEPTIME_SECONDS_KEY, PSTM_PSA_HANDLING_DEFAULT_SLEEPTIME_IN_SECONDS);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering, doing this in the create makes it impossible to update the timeout when the component is active.
Getting it where needed, makes this possible. (I doubt this use case is needed..)
Any specific reasons to do it here and not in the handler thread?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No specific reason, no. Just another case of ctrl+C, ctrl+V.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the overhead of calling celix_bundleContext_getPropertyAsLong() matters much, but that's the only reason I can think of not wanting to do it inside of the loop.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe thread-safety? Hrm.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this solution, if it ever comes up again, we can always take a look at potential risks. I doubt anyone is going to change the timeout at runtime.

@pnoltes
pnoltes merged commit 23d6bce into apache:master Jul 13, 2020
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.

3 participants