Skip to content
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

Merged
merged 7 commits into from
Oct 23, 2023

Conversation

AlvaroVega
Copy link
Member

@AlvaroVega
Copy link
Member Author

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:
Copy link
Member

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

Copy link
Member Author

@AlvaroVega AlvaroVega Oct 20, 2023

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

@AlvaroVega AlvaroVega changed the title update doc update doc about mongo and sth indexes by DM Oct 20, 2023
@AlvaroVega AlvaroVega marked this pull request as ready for review October 20, 2023 09:07
Comment on lines 295 to 297
| DMBYSERVICEPATH | recvTime, entityId, entityType, attrName, attrType, attrValue |
| DMBYENTITY | recvTime, attrName, attrType, attrValue |
| DMBYATTRIBUTE | recvTime, attrType, attrValue |
Copy link
Member

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)

Copy link
Member Author

@AlvaroVega AlvaroVega Oct 20, 2023

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).

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.

Copy link
Member

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?

Copy link
Member Author

@AlvaroVega AlvaroVega Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

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 "..."

Copy link
Member

@fgalan fgalan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fgalan fgalan merged commit 031d7b0 into master Oct 23, 2023
5 checks passed
@fgalan fgalan deleted the task/add_doc_section_mongodb_indexes branch October 23, 2023 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants