Bolt protocol #6

AviAvni · 2023-07-31T10:33:44Z

No description provided.

…into bolt-protocol

swilly22

Overall an interesting PR, I think there's some code relocation to be made but overall pretty straightforward and nicely integrated.

swilly22 · 2023-08-30T12:18:16Z

src/resultset/formatters/resultset_replybolt.c

+	bolt_reply_structure(bolt_client, BST_SUCCESS, 1);
+	bolt_reply_map(bolt_client, 3);
+	bolt_reply_string(bolt_client, "t_first");
+	bolt_reply_int8(bolt_client, 2);
+	bolt_reply_string(bolt_client, "fields");
+	bolt_reply_list(bolt_client, array_len(columns));
+	for(int i = 0; i < array_len(columns); i++) {
+		bolt_reply_string(bolt_client, columns[i]);
+	}
+	bolt_reply_string(bolt_client, "qid");
+	bolt_reply_int8(bolt_client, 0);
+	bolt_client_send(bolt_client);
+}


Please add a comment describing the reply structure here, things such as: t_first and qid aren't clear.

src/resultset/formatters/resultset_replybolt.c

swilly22 · 2023-08-30T12:23:34Z

src/resultset/formatters/resultset_replybolt.c

+		Node *n = v.ptrval;
+		bolt_reply_structure(client, BST_NODE, 4);
+		bolt_reply_int64(client, n->id);
+		uint lbls_count;
+		NODE_GET_LABELS(gc->g, n, lbls_count);
+		bolt_reply_list(client, lbls_count);
+		for(int i = 0; i < lbls_count; i++) {
+			Schema *s = GraphContext_GetSchemaByID(gc, labels[i], SCHEMA_NODE);
+			const char *lbl_name = Schema_GetName(s);
+			bolt_reply_string(client, lbl_name);
+		}
+		const AttributeSet set = GraphEntity_GetAttributes((GraphEntity *)n);
+		int prop_count = ATTRIBUTE_SET_COUNT(set);
+		bolt_reply_map(client, prop_count);
+		// Iterate over all properties stored on entity
+		for(int i = 0; i < prop_count; i ++) {
+			Attribute_ID attr_id;
+			SIValue value = AttributeSet_GetIdx(set, i, &attr_id);
+			// Emit the actual string
+			const char *prop_str = GraphContext_GetAttributeString(gc, attr_id);
+			bolt_reply_string(client, prop_str);
+			// Emit the value
+			_ResultSet_BoltReplyWithSIValue(client, gc, value);
+		}
+		bolt_reply_string(client, "node_0");


can you please explain node_0 ?

swilly22 · 2023-08-30T12:24:02Z

src/resultset/formatters/resultset_replybolt.c

+		bolt_reply_string(client, "relationship_0");
+		bolt_reply_string(client, "node_0");
+		bolt_reply_string(client, "node_0");


Can you please explain these strings: node_0 and relationship_0 ?

swilly22 · 2023-08-30T14:54:06Z

src/module.c

+	char data[4];
+	data[0] = 0x00;
+	data[1] = 0x00;
+	data[2] = version.minor;
+	data[3] = version.major;
+	socket_write(client, data, 4);


Consider moving into a dedicated bolt response / command API

src/module.c

swilly22 · 2023-08-30T14:57:02Z

src/module.c

+		return;
+	}
+
+	client->read_index = 0;


Why are we setting read_index to 0 ? what are we doing with the data we just read ?

src/module.c

src/resultset/formatters/resultset_replybolt.c

+
+	size_t indices = node_count + edge_count - 1;
+	bolt_reply_list(client, indices);
+	for(uint8_t i = 0; i < edge_count; i++) {


This reverts commit a6eeacc.

postpersonality · 2023-09-15T10:11:44Z

tests/tck/features/clauses/create/Create6.feature

Do you run these feature-tests both for Bolt and Redis protocols?

* create vector index structure only * create a float32 vector * bump encoder version * unified encoding decoding versions * support additional sivalue functionality * add support for vector64 * vector byte size * query vector index * use vector byte size when constructing index query * update redisearch submodule * remove support for vector64 * index util for creating and query vector index * remove parser submodule * merge fulltext and exactmatch indices * leaks * wip integrate new syntax * enable test * separate index creation to individual types * index creation is an undoable operation * index level configuration * index creation must create any effects * db.indexes specifies each field types * implement new drop index syntax * refine drop index * test index drop * test index creation * migrate to new index functions * debug flow tests * free fields * switch to index utils * wrong formatting * debugging * update redisearch to latest commit * fix leak * wip * multi type index scan * disable failing test * Bolt protocol (#6) * wip bolt protocol * remove log * remove malloc * clean * handle client state * fixes * handle client response * fix bug * handle chunks * fix remove node label * fix reserve node for merge * socket read exact size * increase buffer size * clean * fix * fix merge * address review * fix delete issue * add tests * address review * fix path * add tiny int support * address reivew * review * parse parameters * fixes * add bolt api * fix leaks * add bolt tck * clean * rename * fix bolt assertions * fix bolt assertions * fix path direction * wip pipeline * wip pipeline * fix endian * better shutdown * wip pipeline * fixes * fix tck check * bolt reply stats * tests * fix * doc * doc * add connection_id * fix crash when client closed * use detached redismodulectx * wait for pull * perf * perf * Revert "perf" This reverts commit a6eeacc. --------- Co-authored-by: Roi Lipman <swilly22@users.noreply.github.com> * perf * fix sanitizer build * bolt port according to redis port * only index entity if updated attribute is part of the index * reduce test memory consumption * update sanitizer build docker image * removed extra encoder * improve bolt perf * fix reset * remove redundant encoder * review * add buffer * address review comments * Update build.yml - latest tag * Update build.yml * Update build.yml * add docker tls support * tls auth clients * fix tests --------- Co-authored-by: AviAvni <avi.avni@gmail.com> Co-authored-by: Guy Korland <gkorland@gmail.com>

* create vector index structure only * create a float32 vector * bump encoder version * unified encoding decoding versions * support additional sivalue functionality * add support for vector64 * vector byte size * query vector index * use vector byte size when constructing index query * update redisearch submodule * remove support for vector64 * index util for creating and query vector index * remove parser submodule * merge fulltext and exactmatch indices * leaks * wip integrate new syntax * enable test * separate index creation to individual types * index creation is an undoable operation * index level configuration * index creation must create any effects * db.indexes specifies each field types * implement new drop index syntax * refine drop index * test index drop * test index creation * migrate to new index functions * debug flow tests * free fields * switch to index utils * wrong formatting * debugging * update redisearch to latest commit * fix leak * wip * multi type index scan * disable failing test * Bolt protocol (#6) * wip bolt protocol * remove log * remove malloc * clean * handle client state * fixes * handle client response * fix bug * handle chunks * fix remove node label * fix reserve node for merge * socket read exact size * increase buffer size * clean * fix * fix merge * address review * fix delete issue * add tests * address review * fix path * add tiny int support * address reivew * review * parse parameters * fixes * add bolt api * fix leaks * add bolt tck * clean * rename * fix bolt assertions * fix bolt assertions * fix path direction * wip pipeline * wip pipeline * fix endian * better shutdown * wip pipeline * fixes * fix tck check * bolt reply stats * tests * fix * doc * doc * add connection_id * fix crash when client closed * use detached redismodulectx * wait for pull * perf * perf * Revert "perf" This reverts commit a6eeacc. --------- Co-authored-by: Roi Lipman <swilly22@users.noreply.github.com> * perf * fix sanitizer build * bolt port according to redis port * only index entity if updated attribute is part of the index * reduce test memory consumption * update sanitizer build docker image * removed extra encoder * improve bolt perf * fix reset * remove redundant encoder * review * add buffer * address review comments * Update build.yml - latest tag * Update build.yml * Update build.yml * wip websocket * fixes * set non blocking * fix handshake fo non blocking client * fix large message write * better write * fix client shutdown and write perf * document buffer * add docker tls support * tls auth clients * fix quantifier null handling --------- Co-authored-by: swilly22 <roi@redislabs.com> Co-authored-by: Roi Lipman <swilly22@users.noreply.github.com> Co-authored-by: Guy Korland <gkorland@gmail.com>

AviAvni added 7 commits July 31, 2023 13:32

wip bolt protocol

6e35d55

Merge branch 'master' into bolt-protocol

1d258b3

remove log

f91f314

Merge branch 'bolt-protocol' of https://github.com/FalkorDB/FalkorDB …

fa09f45

…into bolt-protocol

remove malloc

f1bb763

clean

5ddc467

handle client state

69a5bc8

AviAvni requested a review from swilly22 July 31, 2023 14:42

AviAvni added 13 commits August 1, 2023 09:32

fixes

78086d4

handle client response

096f586

fix bug

f45243a

handle chunks

ab12f8b

fix remove node label

b227576

fix reserve node for merge

514f304

socket read exact size

efbeac6

increase buffer size

82599db

Merge branch 'master' into bolt-protocol

6973c7a

Merge branch 'master' into bolt-protocol

d61ca25

clean

93c2db6

fix

869e9e8

Merge branch 'master' into bolt-protocol

39e0984

swilly22 requested changes Aug 30, 2023

View reviewed changes

AviAvni added 8 commits August 30, 2023 18:30

Merge branch 'master' into bolt-protocol

724d76c

fix merge

a9fa49b

address review

da8643f

fix delete issue

f5b2c89

add tests

edc6b6b

address review

fffff92

fix path

3378a93

add tiny int support

a8ef32e

AviAvni added 17 commits September 3, 2023 11:09

fix path direction

7acb82e

wip pipeline

a5ef26b

wip pipeline

19d6fcd

fix endian

c60ce99

better shutdown

07caf9a

wip pipeline

219c185

fixes

6cf5c4a

fix tck check

1199a5d

bolt reply stats

d7add04

tests

8dd950a

fix

4b5e74e

doc

a13f140

doc

c7ed080

add connection_id

125530b

fix crash when client closed

587a286

Merge branch 'master' into bolt-protocol

1e7a04f

use detached redismodulectx

c59159e

github-advanced-security bot found potential problems Sep 7, 2023

View reviewed changes

AviAvni added 4 commits September 7, 2023 17:31

wait for pull

7cc0e6b

perf

c395d2e

perf

a6eeacc

Revert "perf"

69ecfa6

This reverts commit a6eeacc.

swilly22 changed the base branch from master to 4.0.0-alpha.1 September 11, 2023 05:22

Merge branch '4.0.0-alpha.1' into bolt-protocol

14b6794

swilly22 merged commit 229030f into 4.0.0-alpha.1 Sep 11, 2023
1 of 2 checks passed

swilly22 deleted the bolt-protocol branch September 11, 2023 05:25

swilly22 restored the bolt-protocol branch September 11, 2023 07:11

postpersonality reviewed Sep 15, 2023

View reviewed changes

tests/tck/features/clauses/create/Create6.feature

Copy link

postpersonality Sep 15, 2023

Choose a reason for hiding this comment

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

Do you run these feature-tests both for Bolt and Redis protocols?

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bolt protocol #6

Bolt protocol #6

AviAvni commented Jul 31, 2023

swilly22 left a comment

swilly22 Aug 30, 2023

swilly22 Aug 30, 2023

swilly22 Aug 30, 2023

swilly22 Aug 30, 2023

swilly22 Aug 30, 2023

postpersonality Sep 15, 2023

Bolt protocol #6

Bolt protocol #6

Conversation

AviAvni commented Jul 31, 2023

swilly22 left a comment

Choose a reason for hiding this comment

swilly22 Aug 30, 2023

Choose a reason for hiding this comment

swilly22 Aug 30, 2023

Choose a reason for hiding this comment

swilly22 Aug 30, 2023

Choose a reason for hiding this comment

swilly22 Aug 30, 2023

Choose a reason for hiding this comment

swilly22 Aug 30, 2023

Choose a reason for hiding this comment

postpersonality Sep 15, 2023

Choose a reason for hiding this comment