Opened 9 years ago

Closed 9 years ago

#1055 closed defect (fixed)

fix bug when inserting a new coverage id that is the same name as an existing rasdaman collection

Reported by: Bang Pham Huu Owned by: Bang Pham Huu
Priority: major Milestone: 9.2
Component: petascope Version: development
Keywords: petascope Cc: Dimitar Misev, Alex Dumitru
Complexity: Medium

Description (last modified by Bang Pham Huu)

Modification: This problem happens when a collection has been created in rasdaman database and one using wcst_import to ingest a coverage id with the same as collection name.

Example:

rasql -q "create collection rgb RGBSet" —user rasadmin —passwd rasadmin

wcst_import.sh ingest.json (with "coverage_id": "rgb")

The problem is when I use wcst_import to import some data to rasdaman with coverage id like a, b, c then I drop petascopedb, then I import it again and an error appear. I think this is not consistent in here (it is best that wcst-import not only check in petascopedb but check collection from rasdaman also). Only if both case is not exist then wcst-import can import data or will have error that coverage is exist.

  INFO [14:26:25] RasdamanDefaultCollectionCreator@52: Creating rasdaman collection rgbx3.
 TRACE [14:26:25] RasUtil@149: Executing query CREATE COLLECTION rgbx3 RGBSet
rasj[0] RasRNPImplementation.getResponse: query failed, errNo=955, lineNo=1, colNo=1, token=CREATE
 ERROR [14:26:25] PetascopeInterface@493: Error stack trace:
RasdamanRequestFailed: Error evaluating rasdaman query: 'CREATE COLLECTION rgbx3 RGBSet
	at petascope.PetascopeInterface.handleWcs2Request(PetascopeInterface.java:690)
	at petascope.PetascopeInterface.handleWcsRequest(PetascopeInterface.java:576)
	at petascope.PetascopeInterface.doGet(PetascopeInterface.java:356)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:743)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:856)
	at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:652)
	at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:445)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:137)
	at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:556)
	at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:227)
	at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1044)
	at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:372)
	at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:189)
	at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:978)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:135)
	at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:154)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:116)
	at org.eclipse.jetty.server.Server.handle(Server.java:367)
	at org.eclipse.jetty.server.AbstractHttpConnection.handleRequest(AbstractHttpConnection.java:486)
	at org.eclipse.jetty.server.AbstractHttpConnection.headerComplete(AbstractHttpConnection.java:926)
	at org.eclipse.jetty.server.AbstractHttpConnection$RequestHandler.headerComplete(AbstractHttpConnection.java:988)
	at org.eclipse.jetty.http.HttpParser.parseNext(HttpParser.java:640)
	at org.eclipse.jetty.http.HttpParser.parseAvailable(HttpParser.java:235)
	at org.eclipse.jetty.server.AsyncHttpConnection.handle(AsyncHttpConnection.java:82)
	at org.eclipse.jetty.io.nio.SelectChannelEndPoint.handle(SelectChannelEndPoint.java:628)
	at org.eclipse.jetty.io.nio.SelectChannelEndPoint$1.run(SelectChannelEndPoint.java:52)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:608)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:543)
	at java.lang.Thread.run(Thread.java:745)
 DEBUG [14:26:25] PetascopeInterface@545: Done marshalling Error Report.

Change History (27)

comment:1 by Bang Pham Huu, 9 years ago

However, in fact this case is very rare (not occur frequently), so I think the best is when dropdb there a mechanism to delete all imported collection by petascopedb in Rasdaman also (this is consistent). But if postgresql do not support this functionality then this is outside of my help (Alex, Dimitar have some ideas ?).

comment:2 by Bang Pham Huu, 9 years ago

It looks like hard to know that a collection in SQLite (Rasdaman) is imported with wcst_import (I never check SQLite RASBASE ). If it has then Rasdaman can have a garbage collection that run periodically to delete every coverage that is not in petascopedb anymore.

in reply to:  2 ; comment:3 by Dimitar Misev, 9 years ago

Replying to bphamhuu:

If it has then Rasdaman can have a garbage collection that run periodically to delete every coverage that is not in petascopedb anymore.

That doesn't sound like a good idea.

in reply to:  3 comment:4 by Bang Pham Huu, 9 years ago

Replying to dmisev:

Replying to bphamhuu:

If it has then Rasdaman can have a garbage collection that run periodically to delete every coverage that is not in petascopedb anymore.

That doesn't sound like a good idea.

Yes, I know but this is the worst case, the best is when dropdb then it could trigger to clean all the coverageid in postgresql.

comment:5 by Dimitar Misev, 9 years ago

What do you mean by dropdb? Can you specify which commands exactly do you mean?

in reply to:  5 comment:6 by Bang Pham Huu, 9 years ago

Replying to dmisev:

What do you mean by dropdb? Can you specify which commands exactly do you mean?

Oh sorry, Dropdb is a command of Postgresql to drop database so dropdb petascopedb that make inconsistent.

comment:7 by Dimitar Misev, 9 years ago

So wcst_import fails because a collection in rasdaman with that name already exists.

Then this ticket should be about making the error message from wcst_import more appropriate.
Instead of the stack trace above, it should say:

Failed importing coverage rgbx3; collection with the same name already exists in rasdaman.

comment:8 by Alex Dumitru, 9 years ago

Owner: changed from Bang Pham Huu to Vlad Merticariu
Status: newassigned

@Bang This is not related to WCSTImport, it's related to the WCST component of petascope. These are two different components. WCSTImport only creates the GML files to be inserted using WCST Insert/UpdateCoverage.

@Vlad Maybe we could check if there is already an existing rasdaman collection and rename

comment:9 by Dimitar Misev, 9 years ago

I would not rename or do similar automated workarounds, it's safer to be strict and say there is such a collection already - just rename your coverage or delete the one in rasdaman.

comment:10 by Bang Pham Huu, 9 years ago

Owner: changed from Vlad Merticariu to Bang Pham Huu

I thought I can handle this as Dimitar's suggestion, so I would take this ticket as it is good for me to view Petascope. As existsCoverageName only can return true/false so it is not enough and need to refactor (I see 3 cases in here):

+ 0: coverage is not exist in ps_coverage and rasdaman. (have done)
+ 1: coverage is exist in ps_coverage → also in rasdaman. (have done)
+ 2: coverage is not exist in ps_coverage, but in rasdaman. (refactor)

and I will see WCST_Import then should it need to be added the handler with case 2 (throw Exception that user must remove data by rasql before using WCST_Import) while case 0 is Insert and case 1 is Updated has been done well.

comment:11 by Alex Dumitru, 9 years ago

Owner: changed from Bang Pham Huu to Vlad Merticariu

@Bang Please don't reassign tickets on a whim. This is not a wcst_import issue and should not be treated there.

@Dimitar I don't see any relation between collection names and coverages. Somebody might have created a collection in rasdaman for a different purpose, we can't expect somebody working on the petascope level to care about this (they expect a coverage named X and don't care what is underneath the OGC services). Just my 2 cents.

comment:12 by Bang Pham Huu, 9 years ago

@Alex: Sorry for doing this, ok, then I will leave it for Vlad. Thanks.

comment:13 by Dimitar Misev, 9 years ago

Right, it's a WCS-T issue.

I thought WCS-T was supposed to append some random string in case the coverage name exists as a collection in rasdaman already? We had a discussion on this somewhere but I can't find it.

comment:14 by Bang Pham Huu, 9 years ago

may be Dimitar has mentioned this in [rasdaman-dev] wcst_import: more meaningful type names. Also agree with Alex that using Petascope and Rasql to import is different ways.

I agree that it would be better to have more matching type names. 
We can always append some random bits in case there's a type name clash.
Would you open a ticket?

Dimitar

comment:15 by Bang Pham Huu, 9 years ago

I think random string is hard to manage database. I would want to have this:

+ Using WCST-Import with coverage_id in recipe (for example rgb, mr) then it has postfix _ps (petascope) in ps_coverage and rasdaman (rgb_ps, mr_ps).
+ Using Rasql import then only collection name in rasdaman (rgb, mr).

Then user will don't want to modify coverageId_ps by rasql and it is avoid collision Name between coverage and collection when import data by WCST-Import as in here http://rasdaman.org/ticket/1029

comment:16 by Dimitar Misev, 9 years ago

That's not a good strategy Bang. This is better in my opinion:

  • ingestion of coverage with name rgb
    1. if collection rgb doesn't exist in rasdaman, then the collection linked to this coverage will be called rgb
    2. otherwise, if collection rgb already exists in rasdaman, then the collection linked to this coverage will be rgb_{random bits}
Last edited 9 years ago by Dimitar Misev (previous) (diff)

comment:17 by Vlad Merticariu, 9 years ago

Some time ago we have decided to keep coverage names in sync with corresponding collection names (it was a request from our users, as it makes things clear and easy to handle for them).

If a collection with a name exist (but not a coverage with that name), I agree with Dimitar, we should throw an exception.

The alternative is adding a prefix to the name of the coverage for creating the collection name, but this would create even more confusion among our users (think about having coverage MyCoverage and collections MyCoverage and some_prefix_MyCoverage). Then we would need to explain this behavior, basically adding an exception to our rule of keeping names in sync.

In conclusion, for kipping it simple, I suggest we stick to same name policy for coverages and corresponding collections. When this is not possible ⇒ throw an exception explaining the cause.

Last edited 9 years ago by Vlad Merticariu (previous) (diff)

comment:18 by Vlad Merticariu, 9 years ago

Btw Bang, manually dropping petascopedb will leave you with a populated RASBASE, if you want to make sure both metadata and data are dropped you can use a DeleteCoverage request with parameter coverageId=id1,id2,idN.

Alternatively, you should drop both petascopedb and RASBASE.

in reply to:  17 comment:19 by Dimitar Misev, 9 years ago

Replying to vmerticariu:

If a collection with a name exist (but not a coverage with that name), I agree with Dimitar, we should throw an exception.

Please ignore my earlier comment, I haven't completely understood the issue. I agree with Alex - if you import a coverage with WCS-T and a coverage with the same name doesn't exist, then the coverage should be ingested irrelevant of whether rasdaman contains a collection with the same name or not. The user should not care what is in rasdaman, the interface to him is petascope which has coverages. See comment:16 for an idea on how to implement this.

comment:20 by Dimitar Misev, 9 years ago

Owner: changed from Vlad Merticariu to Bang Pham Huu

How about this ticket Bang?

comment:21 by Bang Pham Huu, 9 years ago

@Dimitar: Thanks, I'll do it as Vlad, Alex also discussed about the problem and where can fix it.

comment:22 by Bang Pham Huu, 9 years ago

I will rename rasdaman collection as Dimitar's opinion like this when the coverage id does not exist in petascopedb but in rasdaman collection.

the postfix is datetime when import data instead of random strings. What do you think?

rgb  ->  rgb (petascopedb)  -> rgb_2016_01_06_10_19_20 (rasdaman collection)
Version 0, edited 9 years ago by Bang Pham Huu (next)

comment:23 by Bang Pham Huu, 9 years ago

Description: modified (diff)
Keywords: petascope added; wcst_import error removed
Milestone: 9.1.x9.2
Summary: wcst_import error with creating new coverage that has been dropped beforeerror when inserting a new coverage id that is the same name as an existing rasdaman collection

comment:24 by Peter Baumann, 9 years ago

grammar error:
+ public static final ExceptionCode CollectionExists = new ExceptionCode("CollectionExists", "Collection name already exist in rasdaman");

missing ":"
+ public static final String EXCEPTION_TEXT = "Error collection name exists already, cannot recreate it: $query";

patch does not contain a test.

summary (minor issue):
"Summary: Problem when one created a collection name and other ingest new coverage with same id." Not sure what this should mean.

Last edited 9 years ago by Peter Baumann (previous) (diff)

comment:25 by Dimitar Misev, 9 years ago

For summary you can take the ticket subject: "fix bug when inserting a new coverage id that is the same name as an existing rasdaman collection".

Another patch on wcst_import testing (that is to be uploaded) contains the test for this one as well, I made the same remark in the review. There is no existing test on wcst_import so the test could not be included in this patch.

comment:26 by Bang Pham Huu, 9 years ago

Summary: error when inserting a new coverage id that is the same name as an existing rasdaman collectionfix bug when inserting a new coverage id that is the same name as an existing rasdaman collection

@Prof. Peter: A patch will be added what you want now.
@Dimitar: Thanks for your idea to put the test for this problem in ticket 1128 (wcst_import test) and more meaningful ticket name.

comment:27 by Bang Pham Huu, 9 years ago

Resolution: fixed
Status: assignedclosed

The patch was accepted, I'll close here.

Note: See TracTickets for help on using tickets.