#1055 closed defect (fixed)

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

Reported by: bphamhuu Owned by: bphamhuu
Priority: major Milestone: 9.2
Component: petascope Version: development
Keywords: petascope Cc: dmisev, mdumitru
Complexity: Medium

Description (last modified by bphamhuu)

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 Changed 23 months ago by bphamhuu

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 follow-up: Changed 23 months ago by bphamhuu

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.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 23 months ago by 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.

comment:4 in reply to: ↑ 3 Changed 23 months ago by bphamhuu

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 follow-up: Changed 23 months ago by dmisev

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

comment:6 in reply to: ↑ 5 Changed 23 months ago by bphamhuu

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 Changed 23 months ago by dmisev

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 Changed 23 months ago by mdumitru

  • Owner changed from bphamhuu to vmerticariu
  • Status changed from new to assigned

@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 Changed 23 months ago by dmisev

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 Changed 23 months ago by bphamhuu

  • Owner changed from vmerticariu to bphamhuu

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 Changed 23 months ago by mdumitru

  • Owner changed from bphamhuu to vmerticariu

@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 Changed 23 months ago by bphamhuu

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

comment:13 Changed 23 months ago by dmisev

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 Changed 23 months ago by bphamhuu

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 Changed 23 months ago by bphamhuu

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 Changed 23 months ago by dmisev

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 23 months ago by dmisev (previous) (diff)

comment:17 follow-up: Changed 23 months ago by vmerticariu

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 23 months ago by vmerticariu (previous) (diff)

comment:18 Changed 23 months ago by vmerticariu

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.

comment:19 in reply to: ↑ 17 Changed 23 months ago by dmisev

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 Changed 21 months ago by dmisev

  • Owner changed from vmerticariu to bphamhuu

How about this ticket Bang?

comment:21 Changed 21 months ago by bphamhuu

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

comment:22 Changed 21 months ago by bphamhuu

I will change coverage_name in 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)
Last edited 21 months ago by bphamhuu (previous) (diff)

comment:23 Changed 21 months ago by bphamhuu

  • Description modified (diff)
  • Keywords petascope added; wcst_import error removed
  • Milestone changed from 9.1.x to 9.2
  • Summary changed from wcst_import error with creating new coverage that has been dropped before to error when inserting a new coverage id that is the same name as an existing rasdaman collection

comment:24 Changed 21 months ago by pbaumann

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 21 months ago by pbaumann (previous) (diff)

comment:25 Changed 21 months ago by dmisev

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 Changed 21 months ago by bphamhuu

  • Summary changed from error when inserting a new coverage id that is the same name as an existing rasdaman collection to fix 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 Changed 21 months ago by bphamhuu

  • Resolution set to fixed
  • Status changed from assigned to closed

The patch was accepted, I'll close here.

Note: See TracTickets for help on using tickets.