-
Notifications
You must be signed in to change notification settings - Fork 104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update doc about mongo and sth indexes by DM #2318
Conversation
and maybe something about indexing procedure (re indexing) it would be nice |
@@ -286,6 +287,25 @@ If `data_model=dm-by-entity` and `attr_persistence=column` then `NGSIMongoSink` | |||
|
|||
[Top](#top) | |||
|
|||
### <a name="section1.4"></a>MongoDB Indexes | |||
|
|||
Since version 3.0.0 of cygnus index named `cyg_agg_opt` is created in this way but depending on datamodel: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should both indexed (cyg_agg_opt and cyg_raw_opt) be mentioned in both sinks (ngsi_mongo_sink.md and ngsi_sth_sink.md)?
As far as I remember (although I may be wrong) each index is used by one sink but I don't remember if it is this way
- mongo sink -> cyg_agg_opt
- sth sink -> cyg_raw_opt
or
- mongo sink -> cyg_raw_opt
- sth sink -> cyg_agg_opt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should both indexed (cyg_agg_opt and cyg_raw_opt) be mentioned in both sinks (ngsi_mongo_sink.md and ngsi_sth_sink.md)?
As far as I remember (although I may be wrong) each index is used by one sink but I don't remember if it is this way
- mongo sink -> cyg_agg_opt
- sth sink -> cyg_raw_opt
or
- mongo sink -> cyg_raw_opt
- sth sink -> cyg_agg_opt
@fgalan you are right!. MongoSink creates Raw table and STHSInk creates Aggregate table.
Fixed in 1c9f22f and 698d5be
| DMBYSERVICEPATH | recvTime, entityId, entityType, attrName, attrType, attrValue | | ||
| DMBYENTITY | recvTime, attrName, attrType, attrValue | | ||
| DMBYATTRIBUTE | recvTime, attrType, attrValue | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should use the names that are used in configuration by users (i.e dm-by-...
). I understand that the DM...
being used are the ones from the code (so, somehow "hidden" by regular users).
In addition, what about the other DMs (from https://github.com/telefonicaid/fiware-cygnus/blob/master/doc/cygnus-ngsi/flume_extensions_catalogue/ngsi_postgresql_sink.md#section1.2 it seems we have 11 DMs, but this table only includes 3)? If they don't involve indexes maybe we should add a sentence like this:
In the case of other datamodels than the ones included in the table above (including the default behaviour when no datamodel is set) cygnus doesn't create any index.
(Same applies to the modification done in PR telefonicaid/fiware-sth-comet#624. Maybe a follow up PR in that repo should be done)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should use the names that are used in configuration by users (i.e
dm-by-...
). I understand that theDM...
being used are the ones from the code (so, somehow "hidden" by regular users).
Done in 49a579c
In addition, what about the other DMs (from https://github.com/telefonicaid/fiware-cygnus/blob/master/doc/cygnus-ngsi/flume_extensions_catalogue/ngsi_postgresql_sink.md#section1.2 it seems we have 11 DMs, but this table only includes 3)? If they don't involve indexes maybe we should add a sentence like this:
In the case of other datamodels than the ones included in the table above (including the default behaviour when no datamodel is set) cygnus doesn't create any index.
(Same applies to the modification done in PR telefonicaid/fiware-sth-comet#624. Maybe a follow up PR in that repo should be done)
IMHO other datamodels does not apply to these sinks and can not be configured with they.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO other datamodels does not apply to these sinks and can not be configured with they.
What does it happen if the user try to user one of such DM with these sinks? Cygnus reports error and refuse to start?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A configuration error should be reported by sink.
https://github.com/telefonicaid/fiware-cygnus/blob/0b16babe87beb25efd5c6b8e40845683e24f3fc2/cygnus-ngsi/src/main/java/com/telefonica/iot/cygnus/sinks/NGSIMongoBaseSink.java#L241C31-L241C60
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thus, I'd suggest to add the following note just after the table in documentaiton:
Note that attempting to use any other datamodel different from these ones will cause a "Unknown data model" ERROR in log traces. In addition, note that default datamodel for STH/Mongo sink is ...
completing the missing "..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
#2317