For the interested in ArcSDE: read below, nice chat pasted. On Thursday 15 May 2008 09:22:01 pm Jody Garnett wrote: > Gabriel Roldán wrote: > > okay, I'm down to 3 failures: > > - testDisjointFilter > > So this one is actually serious? Do we have a plan ... or can we make a > plan? its as serious as a missbehaving spatial filter could be. That is not so much given the bigger fish we have to deal with > > > - testInsertGeometries and testConstructShapePolygon are due to a defect > > in how polygons are mapped to the double[][][] the sde java api uses. > > > > All those are beasts I could overcome independently of the command queue > > stuff, so I'll be working on the later now. > > > > What I want to do: > > start moving stuff that needs SeConnection, SeLayer, etc to be executed > > by Session.execute(Command) > > May imply Session.createXYZ() should no longer be needed at the end of > > the day > > Session has a few other useful methods; like for getting an existing > table (and making use of a cache of SeTable associated with the > connection). As such we may need to allow for: Command.execute( > SeConnection, Session )? > > So I agree we have two separate tasks ahead: > - You can introduce command queue internally for each Session (now that > we are completely isolated) > - I can move the "metadata" methods; creating a FeatureType etc... over > to ArcSDEDataStore.getConnection( Command ) , and then work on reusing a > single Session in my ArcSDEMinimalDataStore subclass > All these and more addressed on the bellow pasted chat:
[21:20:44] Jody Garnett: Hi Gabriel [21:20:52] … are you working on arcsde now? [21:20:59] Gabriel Roldán: hi [21:21:10] … _right now_ no, but I have some uncommited stuff [21:21:23] … uncommited, un-compiling, you know the feeling [21:21:28] Jody Garnett: yeah I do [21:21:31] Gabriel Roldán: wanna coordinate? [21:21:33] Jody Garnett: replying to your message now [21:21:35] … yeah we should [21:21:59] Gabriel Roldán: mostly because by your peak working time I may be sleeping :) [21:24:08] Jody Garnett: so if you have uncommitted work I am likely to stomp all over it right? give you lots of conflicts... [21:24:24] Gabriel Roldán: if you updated for my last commits [21:24:49] … I don't think I'll loose anything of too much value, actually I'm having a lot of thinking [21:24:58] … do you have a minute to talk about it? [21:24:58] Jody Garnett: okay [21:24:59] … sure [21:25:09] Gabriel Roldán: problem is with granularity [21:25:13] … our idea is great [21:25:30] … but at the time of implementing the commands, we get some spagettis [21:25:34] … example: [21:25:39] … FeatureStore.addFeatures [21:25:46] … should be atomic, right? [21:26:01] Jody Garnett: it depends [21:26:06] Gabriel Roldán: but it uses FeatureWriter.write, which should also be, and should call Session.execute by itself [21:26:08] Jody Garnett: for Transaction AUTO_COMMIT it may not be [21:26:19] Gabriel Roldán: right [21:26:25] … my fear is as follows: [21:26:49] … for Transaction.AUTO_COMMIT, I can just do nothing in addFeatures and let FeatureWriter.write issue a command per write [21:26:51] … joy [21:27:01] … for non autocommit, hmmm... [21:27:24] … before I was using the connection lock to hold on until it finishes [21:27:36] … but external locking is going away [21:27:51] … I could issue a single command for the whole addFeatures method body [21:28:04] … but it'll get nested commands in featureWriter.write? [21:28:09] … am I explaining myself well? [21:28:47] … I mean, this could or could not be a problem, depending on how execute(Command) works [21:28:57] … but given how its intended to work... it will be [21:29:07] … deadlock prone [21:29:26] Jody Garnett: you are [21:29:29] … thinking [21:29:38] … I thought the idea was to go all fine grained here [21:29:54] … so feature writer would make a single "command" per feature to write [21:30:27] … Access to the SeConnection is just being shared, by who ever is using the command queue [21:30:38] Gabriel Roldán: yup, that's the idea [21:30:42] Jody Garnett: no reason to send a single big command for addFeatures? [21:30:49] Gabriel Roldán: but then I wonder about the whole transaction atomicity [21:30:54] … for example [21:30:55] Jody Garnett: or ... [21:31:02] … use two different feature writers; one for addFeatures (ie bulk operation) [21:31:11] … and another for adding one by one with user interaction [21:31:44] Gabriel Roldán: yeah I thought about that too [21:31:48] Jody Garnett: I don't think transaction enters into it at this stage? ie if someone commits when their is existing work being done ... we don't help or hinder. [21:31:54] Gabriel Roldán: was just trying to figure out how to write less code [21:32:09] Jody Garnett: well less code right now is to issue a command per Feature [21:32:18] … and treat the bulk data load as an optimization... [21:32:34] Gabriel Roldán: the bulk data load should be atomic afaik [21:32:50] Jody Garnett: note to write less code delegate to a single Writer.addFeature method; and then override it in a subclass. [21:32:58] Gabriel Roldán: ie, two threads, the first call addFeatures, the second getFeatures [21:33:06] … the second shall wait until the first one finishes [21:33:23] Jody Garnett: well with concurrency in the mix it is an open question [21:33:33] … right now with jdbc [21:33:40] … if you are doing this stuff with 2 threads [21:33:43] Gabriel Roldán: think of what udig expects [21:33:46] Jody Garnett: on the same transaction [21:33:51] … you would get mixed results... [21:33:57] Gabriel Roldán: okay [21:34:06] … that's like choosing a transaction isolation level [21:34:12] Jody Garnett: ie I don't think addFeaures promissed anyone it was atomic [21:34:14] … yes it is. [21:34:26] Gabriel Roldán: it didn't [21:34:32] … but uDig expects it, afaik [21:34:49] … hmm... thinking about it that's more a udig problem [21:34:50] Jody Garnett: I think we do try; ie addFeatures for Transaction AUTO_COMMIT; often starts up a transaction, does the work, and then commits for JDBC datastores. [21:35:03] … but shapefile for example offers no such thing. [21:35:06] Gabriel Roldán: like in it sends like 3 concurrent threads: - addFeatures - renderer - selection [21:35:11] … almost at the same time [21:35:32] Jody Garnett: well the "renderer" should be kicked by the addFeature event being fired off [21:35:48] Gabriel Roldán: really?... [21:35:56] Jody Garnett: right now it may be done by a delay; but that delay can be removed (and should be removed) [21:35:57] … really [21:36:05] Gabriel Roldán: I sort of remember udig calling some sort of canvas.refresh [21:36:06] Jody Garnett: this is why I keep saying that feature events matter :-) [21:36:25] … we need them to work; it is how the renderer is supposed to recognize when stuff changes .. by trusting the datastore implementor. [21:36:34] Gabriel Roldán: okay, its releafing to me [21:36:44] Jody Garnett: well we can remove canvas.refresh [21:36:56] … and have something that listens to the data call refresh [21:37:00] … when a solid event is thrown. [21:37:15] Gabriel Roldán: soooo... we can just assume datastores are to be thread safe, but bulk operations are non blocking [21:37:26] Jody Garnett: There are plenty of hacks to remove as geotools is cleaned up.... [21:37:28] Gabriel Roldán: so take care at client code depending on what you want to achieve [21:37:42] Jody Garnett: you are correct [21:37:51] … many of the operations return you a data structure [21:37:59] … that will do the work as your iterate through it [21:38:56] Gabriel Roldán: okay, so what have you been working on, because I got almost all tests passing, all the ones that matter for sure, and switched FeatureSource (read only) to use Session.execute [21:40:29] … ie, what do you want to commit [21:42:00] Jody Garnett: I have nothing right now I want to commit [21:42:18] Gabriel Roldán: ah ok, so I missunderstood [21:42:20] Jody Garnett: but I was planning on working hard on ArcSDEDataStore.getConnection( Command ) [21:42:36] … but if you have uncommitted work I would probably mess it up? [21:42:40] Gabriel Roldán: hmmm odd name though [21:43:17] … and what's in your opinion the difference between ds.getConnection(Command) and Session.execute(Command) [21:43:26] … getConnection(Command) seems like a mistake to me [21:44:01] Jody Garnett: we need to rename getCommand to make sense [21:44:21] … Session.execute( Command ) is the "back end" [21:44:32] … ie a contract with the connectino pool and SeConnection and all that ... [21:44:35] … the queue stuff. [21:44:47] Gabriel Roldán: you want to route through ArcSDEDataStore.execute(Command)? [21:44:59] Jody Garnett: ds.getConnection( Command ) and ds.getConnection( Command, Transaction ) [21:45:02] … is the "front end" [21:45:11] … what the datastore methods call [21:45:20] … the first is for "read-only" (any Session will do) work [21:45:37] … and the second is for "read-write" (specific Session needed, the one associated with the transaction) [21:45:46] … I would love to have this at the ConnectionPool level [21:46:25] Gabriel Roldán: we can make the fact connections are being pooled an implementation detail now [21:46:30] … and rename ConnectionPool as SessionManager or such [21:47:42] … ie, noone needs to know if the connections are pooled or not, the connection is being supplied to the command [21:46:04] Jody Garnett: but could not move it there because of the way some datastore structure is being used to hold on to the Session [21:46:56] … Gabriel am I making sense? [21:50:43] … (heh, apparently not) [21:51:47] Gabriel Roldán: yeah, that started to bother me too, like ArcSDEQuery holding on the Session [21:51:53] … is that what you mean? [21:51:57] Jody Garnett: yes that is a good idea [21:52:13] … so my "MinimalArcSDEDataStore" can have a Session Manager that has a single connection ... [21:52:48] … um SessionManager is a sad name [21:52:52] … I have a "rule" [21:52:56] Gabriel Roldán: in my understanding, you shouldn't need an extra datastore [21:53:10] … maxConnections=1 should just work [21:53:10] Jody Garnett: whenever someone wants to call an class "XXXManager" they have not tought about the roll the things plays yet [21:53:18] Gabriel Roldán: if we implement the command queue [21:53:39] … right [21:53:46] … XXXManager == ranting [21:54:14] … XXXBrainStorm instead? [21:54:30] Jody Garnett: you are correct; I should not need an extra datastore; but I am going to experiment there first; and we can remove it at the end if "Server" is not up to the task ....Server is the thing we have "Session"s with. [21:54:52] … create a Server with these connection parameter; ask for a Session to do some work ... [21:54:55] … I have had worse ideas. [21:55:27] … but still it is not great [21:55:34] Gabriel Roldán: Server looks good, just as good as Session [21:56:21] … still, whatever we call it, I'm worried the dependencies are too intrincated right now [21:56:35] Jody Garnett: just a second [21:56:40] Gabriel Roldán: I'd like to see more straight lines [21:57:05] Jody Garnett: can we fold ArcSDEConnectionPool into the DataStore? [21:57:11] … it is now a thin wrapper over an ObjectPool [21:57:14] Gabriel Roldán: no [21:57:24] Jody Garnett: why not? [21:57:25] Gabriel Roldán: pool is shared with gridcoverage [21:57:30] Jody Garnett: ah the raster supprt... [21:57:37] Gabriel Roldán: hence the separate entity [21:57:48] … BUT [21:57:59] Jody Garnett: okay so where are we too complicated? mostly on our transaction support [21:58:03] Gabriel Roldán: I'd like to make it just a code share, not a live objects share [21:58:13] Jody Garnett: I think the Transaction.State stuff needs to go into the Connection Pool [21:58:30] Gabriel Roldán: yup, into Server :) [21:59:15] Jody Garnett: There is a few things in ArcTransactionState that are not perfect; ie it is keeping a listenerManager, and a boolean versioned flag [21:59:20] … why is all that needed? [21:59:24] Gabriel Roldán: and make a Server instance a DataStore field, hidden to others [21:59:38] Jody Garnett: thinking [21:59:48] … well FeatureWriter will still need to use it? [22:00:00] Gabriel Roldán: FeatureWriter will need a Session [22:00:01] Jody Garnett: but I see your point; no need for a public getConnection( Command method) [22:00:11] … okay cool [22:00:15] Gabriel Roldán: provided as a constructor dependency [22:00:20] Jody Garnett: yep [22:00:40] … so I am okay with all this; but ArcTransactionState has too many responsibilities as it stands... [22:00:51] Gabriel Roldán: it has [22:00:57] Jody Garnett: now we *can* store more than one Transaction.State on the Transaction [22:01:02] … indeed a lot of my code does this. [22:01:14] Gabriel Roldán: I see, I just didn't know before [22:01:23] … thought the relationship was 1-1 [22:01:27] Jody Garnett: how about one transaction.putState(server, state); to hold the SeConnection [22:01:43] … transaction.putState(this, state2); to hold version stuff? [22:01:48] … nope the Transaction is really just like Spring [22:01:53] … it is a container with a lifecycle [22:02:02] … but I did not have any of those words when I made up the API [22:02:08] … (it was not cool enough yet) [22:02:17] Gabriel Roldán: okay :) [22:02:30] Jody Garnett: So is it late for you now? [22:02:37] … or do you want to have a run at this problem and then turn it over to me? [22:02:46] … I have some other work I can do for a couple hours... [22:03:02] Gabriel Roldán: humm... new ArcSDEDataStore(Server), new ArcSDEFEatureSource(Session), new ArcSDEFeatureeWriter(Session) [22:03:18] … I hate Query btw [22:04:02] Jody Garnett: tinking [22:04:08] … we missed something [22:04:21] … the FeatureSource.getSchema() method [22:04:31] … does not *need* a read-write Session [22:04:36] … ie any one would do [22:04:41] … so it needs the Server as well [22:04:47] … does that make sense? [22:05:19] Gabriel Roldán: I guess I would say otherwise [22:05:32] Jody Garnett: yeah I understand [22:05:36] Gabriel Roldán: if it needs the server, you're in danger of needing two connections [22:05:41] … at a single time [22:05:43] Jody Garnett: um; right now the getFeatureType info is passed in right... [22:06:09] … on the other hand [22:06:15] … say your session is going [22:06:42] … they should grab a new Session; and possibly block... [22:06:50] … ie the FeatureSource will be changing Sessions [22:06:56] … as the setTransaction method is called. [22:07:09] … even when they do that; the getSChema() method should always work [22:07:17] Gabriel Roldán: yup [22:07:23] Jody Garnett: even though getFeatures may block until a Session is actually available... [22:07:32] Gabriel Roldán: the point here is, a transaction holds a session [22:07:41] Jody Garnett: correct [22:07:53] Gabriel Roldán: so if you have more sessions available, lucky you [22:08:02] … otherwise wait [22:08:15] Jody Garnett: and they need the Server to lookup the Session for the Transaction [22:08:17] … new ArcSDEFeatureeWriter(Session) is good however [22:08:22] Gabriel Roldán: but beware it'll be tied for the transaction lifetime [22:08:42] Jody Garnett: actually no; they can change to another Transaction at any point ... [22:09:29] … oh sorry [22:09:30] Gabriel Roldán: when a Session is closed ( and hence returned to the server for recycling)? at Transaction.commit and Transaction.rollback [22:09:38] … Transaction.close, not sure [22:09:41] Jody Garnett: you were saying that ArcSDEFeatureWriter( Session ) is stuck on that session for ever [22:09:44] … yeah. [22:09:57] Gabriel Roldán: the writer is, the store is not [22:10:05] Jody Garnett: correct. [22:10:07] Gabriel Roldán: the store can be set with another transaction [22:10:09] … okay [22:10:32] Jody Garnett: the FeatureStore is just a staging area; it brings together all the information needed so we can get a ArcSDEFeatureWriter( Session ) going... [22:10:39] … so question [22:10:50] … are we making things too difficult [22:10:57] … by trying to make things pretty? [22:11:11] … ie a writer needs a listenerManager as well; so it can issue those events etc... [22:11:13] Gabriel Roldán: beleave me, we're making the clearer [22:11:27] … making _it_ clearer, I mean [22:11:36] Jody Garnett: okay so back on track [22:11:41] … what are we doing [22:11:42] … and who is doing it? [22:11:54] … I am totally over budget on this; mostly due to setting up that ArcSDE instance... [22:12:02] Gabriel Roldán: ditto [22:12:03] Jody Garnett: so I want to clear this one away. [22:12:19] Gabriel Roldán: I have some time to work on it though [22:12:34] … let me clear my mind and propose a set of structured changes [22:12:47] Jody Garnett: okay [22:12:57] … do you want to send email; or skype on this in a couple hours? [22:13:05] … or is there something useful I can do in the meantime [22:13:11] Gabriel Roldán: so I can say Jody: can you take care of ArcTransactionState while I do with Writer? [22:13:14] … or such [22:13:30] Jody Garnett: perhaps move getConnection( Command ) methods to the connection pool. [22:13:32] Gabriel Roldán: you already did, this conversation is helping a lot [22:13:34] Jody Garnett: okay sure [22:13:37] … you organize and think [22:13:42] … and I will go work on something else for a bit. [22:13:47] Gabriel Roldán: wait [22:13:52] Jody Garnett: yep [22:13:56] Gabriel Roldán: getConnection(Command) [22:14:07] … weren't we getting rid of it? [22:14:07] Jody Garnett: yes [22:14:11] … not so much [22:14:17] … we are moving it out of the public api [22:14:21] … and into [22:14:28] … Server.issueCommand( Command ) [22:14:43] … and then [22:14:44] Gabriel Roldán: which in turn grabs a Session and runs the command with it? [22:14:58] Jody Garnett: public abstract class Command { public abstract void execute(Session session, SeConnection connection) throws IOException; } [22:15:14] … ie it is the way we let datastore code work with the raw SeConnection [22:15:21] Gabriel Roldán: okay, I was thinking of passing Session as execute argument anyway [22:15:47] … but the relationshipt between session and connection [22:15:47] Jody Garnett: we have to give them a Session as well so they can make use of the session.chachedTables [22:16:01] Gabriel Roldán: is strong enough as to have Command.execute(Session) [22:16:08] Jody Garnett: it is [22:16:08] Gabriel Roldán: and then Session.getConnection():SeConnection [22:16:13] Jody Garnett: except we have a problem [22:16:21] … I don't want Session.getConnection() to be a public API [22:16:33] … it is okay if it is a package visible method between Server and Session [22:16:46] … and Server can unpack it and pass it into the Command.execute method [22:16:55] Gabriel Roldán: as public as all the internal plugin code is... [22:16:58] Jody Garnett: but I don't want to see anything like Session.unwrap() [22:17:13] … well I think we can make it package visible [22:17:17] … Session and Server are in the same package. [22:17:28] Gabriel Roldán: I doubt it [22:17:42] Jody Garnett: no you don't understand me ... [22:17:46] … um ... [22:17:50] Gabriel Roldán: most commands will be inner anonymous classes [22:17:51] Jody Garnett: org.geotools.arcsde.pool [22:17:56] … contains [22:17:58] … Session [22:18:00] … (and soon) [22:18:01] … Server [22:18:23] … Server.schedule( Command, Transaction ) method is public [22:18:29] … and the implementation can [22:18:33] … a) find the Session [22:18:49] … b) call the package visible Session.getConnection() method (that only Server can see) [22:18:52] … and finally [22:19:03] … c) call command.execute( Session, SeConnection ) [22:19:04] … is that cool? [22:19:28] … note Server.schedule actually places the command onto a queue for thread safety but nobody needs to know about that... [22:19:32] Gabriel Roldán: more or less, it does not make explicit the fact that any Session.createXXX method is using the same connection [22:19:53] … yeah, Server.schedule is clear [22:20:04] Jody Garnett: We can document that in the Command.execute method ... [22:20:12] Gabriel Roldán: I'm not sure about the two arguments [22:20:21] … or rather [22:20:23] Jody Garnett: well we could get away with just SeConnection [22:20:32] … but then we would miss out on the various caches [22:20:43] Gabriel Roldán: with the given connection I can do new SeLayer(connection) myself [22:20:47] Jody Garnett: I could wrap up an SeConnection like you did before, but that would scare me [22:20:57] Gabriel Roldán: instead of using the Session as factory [22:21:17] Jody Garnett: you are correct; within a command you can do new SeLayer( connection ) yourself; we will document that createSeLayer is more fun; because the result will be cached for later. [22:21:42] … note: this is the "weakness" of our design; many of the createXXX methods [22:21:46] … could now be repalced with a command. [22:21:50] … indeed you could ... [22:21:59] … implement them as a command interally [22:22:06] … and then hit the inline code refactoring in eclipse [22:22:07] Gabriel Roldán: so for the command, the session becomes just a convenient factory [22:22:11] Jody Garnett: to get rid of all the createXXX methods. [22:22:22] Gabriel Roldán: we're not calling Session.execute [22:22:25] Jody Garnett: well for the COmmand the session has one useful use... [22:22:43] Gabriel Roldán: and session does nothing else, from the standpoint of the command, than providing it layers and tables [22:22:43] Jody Garnett: getTable( String ) [22:22:52] … correct [22:23:09] … indeed Session.execute( Command ) should be package visible only [22:23:23] … ie only the Server will call that one right? [22:23:33] … oh; I don't suppose it matters... [22:23:50] Gabriel Roldán: correct on package visibility [22:23:59] Jody Garnett: good good [22:24:06] … it is sure fun working with you [22:24:14] Gabriel Roldán: still [22:24:19] Jody Garnett: but lets get this one done; we will have other chances later. [22:24:25] Gabriel Roldán: I feel better with a single Session argument to command [22:24:32] … so there's no magic [22:24:42] Jody Garnett: I don't think the danger is worth it [22:24:49] … it gives people a public way to ignore our API [22:24:53] Gabriel Roldán: Session _holds_ on a connection, provides access to ther sde java api resources [22:24:54] Jody Garnett: and get an SeConnection [22:25:09] … okay [22:25:14] … compromise [22:25:19] Gabriel Roldán: what's the danger? [22:25:24] … not quite getting it [22:25:40] Jody Garnett: Session2 extends Session { SeConnection getConnection(); } [22:25:50] … Commmand.execute( Session2 ) [22:25:53] … ah [22:26:02] Gabriel Roldán: uh? [22:26:02] Jody Garnett: I want the only way to get an SeConnection to be under our control [22:26:10] Gabriel Roldán: me too [22:26:11] Jody Garnett: ie at the end of a queue [22:26:23] … We have Sessions floating around there in the wild [22:26:42] Gabriel Roldán: okay [22:26:43] Jody Garnett: so a public Session.getConnection() method is right out ... [22:26:45] Gabriel Roldán: I understand that [22:26:57] Jody Garnett: ie the kind that can be used by a Command someone implmeents for our datastore. [22:27:07] Gabriel Roldán: but, all the fun Session were meant to provide seems to be moving to Server [22:27:09] Jody Garnett: so how can we do Command? [22:27:25] … just a sec phone call [22:27:31] Gabriel Roldán: np [22:28:28] … and if we'll issue commands through Server, what will need to be floating around is Server instead of Session [22:28:49] … but Session too, since we'll need to attach them to transactions [22:29:53] Jody Garnett: back [22:29:54] … udig support call [22:29:56] … svn moving is killing people ;-( [22:30:22] … thinking [22:30:30] … would Session.schedule( Command ) make more sense? [22:30:32] Gabriel Roldán: put the svn switch command as your skype topic [22:30:41] Jody Garnett: ie people look up the session for their Transaction [22:30:44] … and then schedule a command on it [22:30:49] Gabriel Roldán: yup [22:30:56] Jody Garnett: or use the createXXX() methods which are like scheduling a "ready made" command. [22:31:22] … lets do that then; schedule is a good word; it describes what is going on and lets people know their may be a delay... [22:31:30] … So for Command [22:31:48] … we still need an Command.execute( Session session, SeConnection ) right? [22:31:51] … ie two parameters? [22:31:53] Gabriel Roldán: not sure about schedule, because we're blocking client code until it returns [22:32:06] … scheduling the command is an internal detail? [22:32:40] … I preffer Session.issue(Command) and Command.execute( Session session, SeConnection ) [22:32:54] … better than doing it on Server [22:32:56] … yes [22:33:29] … I concede for the two parameters in the intent of lowering complexity [22:34:28] … happy? [22:34:33] Jody Garnett: yeah you are right; we are blocking so it is not really a schedule [22:34:47] … Session.issue it is... [22:35:13] Gabriel Roldán: okay lets see what I can do in two hours... [22:35:18] Jody Garnett: okay [22:35:23] Gabriel Roldán: matter if I send this chat to the ml? [22:35:26] Jody Garnett: I will be online; tag me if needed [22:35:33] … nope go for it [22:35:43] Gabriel Roldán: k, see ya ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ Geotools-devel mailing list Geotools-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel