[jboss-jira] [JBoss JIRA] (WFLY-1877) Make KeepAliveTimeAttributeDefinition deal with both a simple long and a complex attribute
Brian Stansberry (JIRA)
issues at jboss.org
Mon Feb 3 13:38:28 EST 2014
[ https://issues.jboss.org/browse/WFLY-1877?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12940925#comment-12940925 ]
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
> Make KeepAliveTimeAttributeDefinition deal with both a simple long and a complex attribute
> ------------------------------------------------------------------------------------------
>
> Key: WFLY-1877
> URL: https://issues.jboss.org/browse/WFLY-1877
> Project: WildFly
> Issue Type: Feature Request
> Security Level: Public(Everyone can see)
> Components: Domain Management
> Reporter: Brian Stansberry
> Assignee: Brian Stansberry
> Fix For: 8.0.0.Final
>
>
> This attribute should support either a simple value or a complex one.
--
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
More information about the jboss-jira
mailing list