Re: [jgroups-dev] JGRP-1231 create a Discovery protocol using a javax.sql.DataSource

Hello,
thank you for all the pointers; I've followed almost all, just the
helper class for handling different databases is missing as it seems I
could keep the SQL related code simple enough for the task.

Here is a preview of the work so far:
https://github.com/Sanne/JGroups/commit/62bdc79b9f549bc988f10838836f5acff2124ab6

I'm still missing:
1) writing documentation - I'll have to wait for some help as
JGRP-1273 is making me a blind writer
2) to test the JNDI code - never run that yet.

So this should be ready tomorrow, if you won't be too hard on the
feedback on above patch :)

I've had to change a constructor of PingData, it seemed to make sense
and shouldn't break anything, but please double check it; there's no
hard need to change it, it saves me two lines of code.

Regards,
Sanne

2011/1/17 Bela Ban <[email protected]>:
>
>
> On 1/17/11 11:48 AM, Sanne Grinovero wrote:
>> Hello,
>> I'm implementing this new Discovery protocol which relies on a
>> database, I'm now finally tinkering with it and got some precise
>> questions:
>>
>> 1)About errorhandling
>> as I'm basing this new protocol to FILE_PING, I see that IO errors
>> such as "couldn't write my own Address into the file" are silently
>> swallowed.
>>
>> [see org.jgroups.protocols.FILE_PING.writeToFile(PingData, String) :
>> try {
>>              out=new DataOutputStream(new FileOutputStream(file));
>>              data.writeTo(out);
>>          }
>>          catch(Exception e) {
>>          }
>>          finally {
>>              Util.close(out);
>>          }
>> ]
>>
>> Should this be the case too for my new implementation? I'm wondering
>> if the code is a correct example for me to follow.
>
>
> What would you prefer to do ? Log the exception ? This method is called
> by a thread, and run() cannot rethrow the exception...
>
>
>> 2)About SQL generation, and all crazy dialects.
>> So obviously we need to define a (simple) schema and perform
>> insert/update/delete operations; still I've seen that using the
>> driver's java.sql.DatabaseMetaData
>> to find out basic things such as if the table already exists is quite
>> tricky; to implement it properly I'd have to copy large utility
>> classes from Hibernate or Infinispan
>> which already have comparable features.
>> But even if I did, then we should need a way to create the needed
>> table in some atomic way, as I'd expect this feature to be used
>> by systems starting several nodes at approximately the same time.
>> We could likely live without it, but it seems that many databases have
>> such capabilities, or for some a non critical error would be thrown
>> which we could safely ignore.
>> So to be able to use such capabilities, these are absolutely
>> non-standard SQL syntaxes and have varying approaches, I need to
>> provide a very flexible way:
>> My proposal is to expose the full SQL statement to be performed as an
>> "initialize_sql" configurable property, which follows these rules:
>>
>> # if empty, it's ignored and no schema creation is attempted at
>> startup. People could prepare the database beforehand with the tools
>> they are more comfortable with.
>>
>> # if defined, it's not validated but just sent to the connection at
>> init(). Errors are logged at info level, and not re-thrown.
>>
>> A consequence of exposing the full schema flexibility is that users
>> will need to define also each CRUD statement as a configuration
>> property.
>
>
> Yes. I've done this before, when I wrote the first JBossCache JDBC
> cacheloader. There were SQL statements which created, deleted the table,
> and inserted, updated and removed rows.
>
> I think it's fine to have a create_table_stmt, delete_table_stmt,
> insert_stmt etc... All of them have predefined values, but of course,
> users can override them.
>
>
>> Another downside is that if people actually have an error in schema
>> creation script, and the schema is not created, we don't fail to
>> startup.
>
>
> That's a misconfiguration...
>
>
>> 3)Configuration properties and inheritance of Discovery strategies.
>> The draft we made on JUDCon is having JDBC_PING extend  FILE_PING, but
>> so I'm now inheriting undesired configuration properties, such as the
>> file location.
>> Is there some way to disable the inheritance of this property?
>
>
> Unfortuately, not. I suggest just ignore this property.
>
>
>> Also this new JDBC_PING could be configured in two different ways:
>>
>>    a) by providing the several needed configuration elements to open a
>> JDBC connection (Driver implementation name, login, password, JDBC
>> URL)
>>    b) by providing the JNDI name of an existing DataSource
>>
>> So to have the configuration properties mandate a proper "either-or"
>> configuration, I'd make two different concrete implementations
>> extending a common abstract protocol, naming the second one as
>> DATASOURCE_PING ?
>
>
> I would not do this. I'd add both datasource and the other properties,
> and (in init()) do some sanity checking, to see if we've got all props
> needed...
> This was also done in the JdbcCacheLoader in JBossCache
>
>
> --
> Bela Ban
> Lead JGroups / Clustering Team
> JBoss
>
> ------------------------------------------------------------------------------
> Protect Your Site and Customers from Malware Attacks
> Learn about various malware tactics and how to avoid them. Understand
> malware threats, the impact they can have on your business, and how you
> can protect your company and customers by using code signing.
> http://p.sf.net/sfu/oracle-sfdevnl
> _______________________________________________
> Javagroups-development mailing list
>
>

------------------------------------------------------------------------------
Protect Your Site and Customers from Malware Attacks
Learn about various malware tactics and how to avoid them. Understand 
malware threats, the impact they can have on your business, and how you 
can protect your company and customers by using code signing.
http://p.sf.net/sfu/oracle-sfdevnl
_______________________________________________
Javagroups-development mailing list