[ https://issues.jboss.org/browse/WFLY-1877?page=com.atlassian.jira.plugin.... ] Brian Stansberry commented on WFLY-1877: ---------------------------------------- Reminder to self of what this is all about: emmartins: wrt https://github.com/wildfly/wildfly/pull/4887/files#r5718928 [09:04am] jbossbot: git pull req [wildfly] (open) emmartins WFLY-238: jsr 236 config for ee subsystem management https://github.com/wildfly/wildfly/pull/4887 [09:04am] jbossbot: jira [WFLY-238] Thread pool configuration in EE subsystem [Open (Unresolved) Sub-task, Major, Eduardo Martins] https://issues.jboss.org/browse/WFLY-238 [09:04am] emmartins: there are 3 attributes that this apply [09:05am] emmartins: but one relies on another to specify the unit [09:05am] emmartins: KEEP_ALIVE_TIME [09:05am] emmartins: this matches the params of RI constructors [09:05am] emmartins: but my guess is that wrt to consistency [09:05am] emmartins: we could have all with specific units [09:06am] bstansberry: emmartins: no, please follow the thread subsystem re: keep-alive time [09:06am] emmartins: even the RI code than translates keep alive time to seconds [09:06am] emmartins: so not sure what we gain to have that one change the unit [09:06am] bstansberry: it shouldn't vary from subsystem to subsystem and we are not going to break compatibility in the existing threads uses [09:07am] emmartins: but have thread life time and hang task threashold fixed to seconds and milliseconds [09:08am] bstansberry: emmartins: ah, sorry, i misread. so you were thinking about making them all have separate units [09:09am] bstansberry: emmartins: separate units for all would be better. it's a bit more of a pain, but hopefully not too bad [09:10am] emmartins: I mean define KEEP_ALIVE_TIME as seconds too, and remove the attr KEEP_ALIVE_TIME_UNIT [09:10am] emmartins: that is [09:10am] emmartins: HUNG_TASK_THRESHOLD -> milliseconds [09:11am] emmartins: KEEP_ALIVE_TIME and THREAD_LIFE_TIME -> seconds [09:13am] • bstansberry is pondering [09:16am] bstansberry: the sucky part is the threads KEEP_ALIVE_TIME is a single complex attribute, not a pair of simple attributes [09:20am] bstansberry: ctomc: do you know off the top of your head whether the CLI validates parameters beyond their name? [09:20am] bstansberry: I'm pretty sure it doesn't [09:20am] ctomc: you mean types [09:20am] ctomc: afaik not [09:20am] bstansberry: ctomc: ok, good [09:21am] bstansberry: ctomc: this is re: the complex keep-alive-time attribute in the threads subsystem and others that use it [09:22am] ctomc: i knew this is going to hounts us again [09:22am] bstansberry: as long as clients don't validate, in future we could just make that a simple long and have the server side AD deal with converting the legacy data [09:22am] bstansberry: i.e. the AD accepts the complex attribute and just converts to ms or s [09:23am] ctomc: yeah, and we validate it in AD [09:23am] ctomc: in this case KeepAliveTimeAttributeDefinition [09:25am] emmartins: AD has a validator framework right? [09:26am] emmartins: so isn't just an issue of ensuring the dev uses it when it should? [09:26am] ctomc: yes [09:26am] ctomc: every AD can define validator [09:26am] emmartins: but then, how to validate beyond the ranges [09:26am] ctomc: what do you want to validate? [09:26am] emmartins: how to say admin used 60000 when it should uhave used 60 [09:27am] emmartins: he may really want 60000 [09:27am] jbossbot: new jira [WFLY-1877] Make KeepAliveTimeAttributeDefinition deal with both a simple long and a complex attribute [Open (Unresolved) Feature Request, Major, Brian Stansberry] https://issues.jboss.org/browse/WFLY-1877 [09:28am] emmartins: bstansberry: I am all for a simpler approach, make sure that admins see the unit [09:28am] jbossbot: new git pull req [wildfly] (open) ssilvert WFLY-1840 Put Multi-JSF installer into WildFly build (take 2) https://github.com/wildfly/wildfly/pull/4918 [09:28am] emmartins: and are responsible for setting correct values [09:28am] ctomc: option 1) have default value for unit [09:28am] ctomc: option 2) complain when unit is not passed [09:29am] emmartins: wait wait [09:29am] emmartins: there is no unit selection in some of these [09:29am] emmartins: HUNG_TASK_THRESHOLD -> milliseconds [09:29am] emmartins: THREAD_LIFE_TIME -> seconds [09:30am] bstansberry: emmartins: yeah. the thing i'm debating is adding that, which is kind of ugly because we don't like complex attributes [09:30am] bstansberry: they are a pain to set via the CLI [09:30am] emmartins: my question is really just to additionally make KEEP_ALIVE_TIME as seconds too, removing the KEEP_ALIVE_TIME_UNIT ad [09:31am] bstansberry: sure, i understand [09:31am] emmartins: I think you guys are already going into another level [09:31am] emmartins: [09:31am] bstansberry: it's a question of whether we make this one different from the 3 other uses of keep-alive-time [09:31am] ctomc: [09:31am] bstansberry: and that's bad [09:31am] bstansberry: so i'm exploring how to actually change the 3 other uses to a simple attribute [09:31am] emmartins: threads isn't going to die? [09:31am] ctomc: how common is unit used in any case? [09:32am] bstansberry: while still maintaining compatibility [09:32am] ctomc: bstansberry +1 [09:32am] bstansberry: emmartins: no, threads is not dying [09:32am] ctomc: we need to get rid of threads subsystem in any case [09:32am] ctomc: but yeah emmartins that does not mean threads are going away [09:32am] emmartins: hehe [09:32am] bstansberry: emmartins: it is becoming a common code base for use by other subsystem that need to configure such things [09:32am] emmartins: I thought most want to migrate into ee concurrent stuff [09:32am] bstansberry: so, this exact same problem is in ejb and jca [09:33am] bstansberry: so even if we just deleted threads subsystem, the consistency problem is still there [09:33am] emmartins: doesn't make much sense to manage multiple frameworks for these [09:34am] emmartins: did not ee specs introduced concurrent api to solve that? [09:35am] ctomc: no [09:35am] ctomc: that is different thing [09:35am] ctomc: it is welcome change [09:35am] ctomc: but this API was introduced to provide users a way to write concurrent stuff [09:35am] bstansberry: emmartins: please define keep-alive-time in ms and leave it at that. with that we have a path forward to convert the behavior of the other existing keep-alive-time attributes without losing precision [09:35am] ctomc: does not help us much in core wildfly as such [09:38am] emmartins: bstansberry: the RI internals convert to seconds [09:39am] emmartins: a thread keep alive in ms probably overkill measure units [09:45am] th_janssen left the chat room. (Read error: Connection reset by peer) [09:45am] ctomc: emmartins just use simple attributes, but note in attribute description what is expected unit for that [09:45am] ctomc: from my point of view [09:45am] ctomc: it is same as with servlet's <session-timeout>30</session-timeout> [09:45am] ctomc: if you dont read xsd or spec you don't know it is minutes [09:46am] emmartins: keep-alive-time-in-seconds ? [09:47am] bstansberry: emmartins: no [09:47am] bstansberry: keep-alive-time, expressed in ms, with text in the description saying it is ms and also .setMeasurementUnit(MeasurementUnit.MILLISECONDS) [09:48am] emmartins: ok [09:48am] bstansberry: if you think all our keep-alive-times should be in seconds and not ms, please file a subtask under WFLY-1877 [09:48am] bstansberry: emmartins: it's a valid question, but I need to deal with other stuff today [09:49am] emmartins: not really important [09:49am] emmartins: as long as its clear [09:49am] emmartins: to who sets the value [09:49am] bstansberry: emmartins: thanks for raising this discussion. just blindly expanding what threads was doing to these other attributes would have been kind of yuck [09:50am] emmartins: np [09:50am] bstansberry: emmartins: right, clarity is important. that's where the text description and the setMeasurementUnit come in. they function like javadoc [09:51am] bstansberry: xsd documentation is good too, which you should add [09:51am] emmartins: y -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira