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

Lock on write proc #1573

Merged
merged 2 commits into from
Feb 13, 2021
Merged

Lock on write proc #1573

merged 2 commits into from
Feb 13, 2021

Conversation

swilly22
Copy link
Contributor

Resolves #1570

Copy link
Contributor

@jeffreylovitz jeffreylovitz left a comment

Choose a reason for hiding this comment

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

👍 Looks good! A few small comments.

@@ -85,34 +85,47 @@ int Schema_AddIndex(Index **idx, Schema *s, const char *field, IndexType type) {
return INDEX_OK;
}

int Schema_RemoveIndex(Schema *s, const char *field, IndexType type) {
static int _schema_RemoveExactMatchIndex(Schema *s, const char *field) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit - _Schema_RemoveExactMatchIndex

}

return INDEX_OK;
}

static int _schema_RemoveFullTextIndex(Schema *s) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit - _Schema_RemoveFullTextIndex

ResultSet_IndexDeleted(result_set, res);
Schema *s = GraphContext_GetSchema(gc, label, SCHEMA_NODE);

if(s != NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch!

Comment on lines +42 to +43
if(GraphContext_AddIndex(&idx, gc, label, field, IDX_FULLTEXT) == INDEX_OK) {
res = INDEX_OK;
Copy link
Contributor

Choose a reason for hiding this comment

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

res = GraphContext_AddIndex(&idx, gc, label, field, IDX_FULLTEXT);
if(res != INDEX_OK) break;

FWIW the older approach had the benefit of resolving the schema only once, but I don't mind this change if you think it's a better abstraction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not only abstraction, but this way will make sure the statistics are updated, we can't break here as there might be several fields being added out of which some might already exists. we want to construct only if the user presented a new field

@swilly22 swilly22 merged commit 84cb546 into master Feb 13, 2021
@swilly22 swilly22 deleted the lock-on-write-proc branch February 13, 2021 08:02
swilly22 added a commit that referenced this pull request Feb 14, 2021
* write lock when invoking write procedure

* replicate index modifications
This was referenced Feb 14, 2021
swilly22 added a commit that referenced this pull request Feb 16, 2021
* Avoid postponed array (#1569)

* WIP

* persist projected values

* remove emit record

* Lock on write proc (#1573)

* write lock when invoking write procedure

* replicate index modifications

* Use file streaming endpoint to parse queries (#1572)

* Use file streaming endpoint to parse queries

* Revert change to default parse buffer size

* Address PR comments

Co-authored-by: Roi Lipman <swilly22@users.noreply.github.com>

Co-authored-by: Jeffrey Lovitz <jeffrey.lovitz@gmail.com>
pnxguide pushed a commit to CMU-SPEED/RedisGraph that referenced this pull request Mar 22, 2023
* write lock when invoking write procedure

* replicate index modifications
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.

fulltext index creation isn't replicated
2 participants