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

Error when encoding value breaks connection permanently #367

Closed
jvf opened this issue May 24, 2024 · 5 comments · Fixed by #379
Closed

Error when encoding value breaks connection permanently #367

jvf opened this issue May 24, 2024 · 5 comments · Fixed by #379
Labels

Comments

@jvf
Copy link
Contributor

jvf commented May 24, 2024

I noticed that inserting with a type error in the inserted values breaks a connection permanently (output shortened):

$ iex -S mix
==> utils
Erlang/OTP 26 [erts-14.2.1] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit]
Interactive Elixir (1.15.7) - press Ctrl+C to exit (type h() ENTER for help)

iex(1)> Application.spec(:xandra)[:vsn]
~c"0.19.0"

# we start a connection with max_concurrent_requests_per_connection: 1 to see the problem faster
iex(2)> {:ok, conn} = Xandra.start_link(authentication: {Xandra.Authenticator.Password, [username: "cassandra", password: "cassandra"]}, max_concurrent_requests_per_connection: 1)

# prepare keyspace and table
iex(3)> Xandra.execute(conn, "CREATE KEYSPACE test WITH REPLICATION = {'class': 'SimpleStrategy', 'replication_factor': 1}")
iex(4)> Xandra.execute(conn, "CREATE TABLE test.participants (name text PRIMARY KEY, number int)")

# normal insertions in fast sequential order are no problem:
iex(5)> Enum.each(1..100, fn x -> Xandra.execute(conn, "INSERT INTO test.participants (name, number) VALUES (:name, :number)", %{"name" => {"text", "john#{x}"}, "number" => {"int", x}}) end)
:ok

# now we do one erroneous insertion (the `number` value is incorrectly given as string):
iex(6)> Xandra.execute(conn, "INSERT INTO test.participants (name, number) VALUES (:name, :number)", %{"name" => {"text", "john"}, "number" => {"int", "123"}})
** (FunctionClauseError) no function clause matching in Xandra.Protocol.V5.encode_value/2
[...]

# after one error the connection is broken permanently, even for correct insertions
iex(6)> Xandra.execute(conn, "INSERT INTO test.participants (name, number) VALUES (:name, :number)", %{"name" => {"text", "jane"}, "number" => {"int", 124}})
{:error,
 %Xandra.ConnectionError{
   action: "check out connection",
   reason: :too_many_concurrent_requests
 }}

This looks like a bug to me. The same works with different max_concurrent_requests_per_connection settings but you have to make more erroneous calls. It looks like each call that produces an encoding error "consumes" one of the concurrent requests, presumably by using up one a stream id without releasing it?

I also tried the same experiment with Xandra.execute(conn, "SELECT * FROM non_existing_table") but this does not break the connection.

@whatyouhide
Copy link
Owner

Ok interesting, yeah definitely a bug, thanks for reporting @jvf. Any chance you or @harunzengin could give this a try? I was on vacation and I won't have time next week (catch up week!) so it'd be a while til I could get to this.

@jvf
Copy link
Contributor Author

jvf commented Jun 7, 2024

@harunzengin agreed to look into this.

@siduniyal21
Copy link

siduniyal21 commented Dec 14, 2024

Any updates here @jvf ? , I also ran into the same issue

@jvf
Copy link
Contributor Author

jvf commented Jan 15, 2025

I looked into this. I think the problem is this: When we execute a query we first allocate a stream id. Then the encoding fails due to the wrong type. But when an encoding error occurs during value serialization, the stream ID is never properly released. This stream id is permanently unusable, resulting in permeant too_many_concurrent_requests errors.

I tried a bit to fix releasing the stream ids correctly in case of error, but I have not yet found the correct way to do it.

@whatyouhide If you have any hints I am happy to try to implement it.

@jvf
Copy link
Contributor Author

jvf commented Jan 15, 2025

I think I found a way to do it, I will provide a PR soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants