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

api: queue works with connection_pool #212

Merged
merged 4 commits into from
Sep 30, 2022

Conversation

oleg-jukovec
Copy link
Collaborator

@oleg-jukovec oleg-jukovec commented Aug 25, 2022

The patchet allows to use the queue subpackage with the connection_pool subpackage:

  1. It adds missed Execute* methods to ConnectionPool.
  2. It adds ConnectorAdaptor to the connection_pool subpackage to be able to use ConnectionPool as Connector.

Finally, it adds an example how-to use a Queue with ConnectionPool.

I didn't forget about (remove if it is not applicable):

Related issues:

Closes #176

@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-176-queue-loves-pool branch 2 times, most recently from d774023 to cde62df Compare August 26, 2022 14:31
@oleg-jukovec
Copy link
Collaborator Author

oleg-jukovec commented Aug 26, 2022

This pull request is ready for review in general, but I'll wait for #210 to spend less time on rebase.

Also there is a bug in the queue that needs to be fixed to make tests successful.
tarantool/queue#185

@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-176-queue-loves-pool branch 2 times, most recently from e1bf9cf to 679ab77 Compare September 12, 2022 13:08
@oleg-jukovec
Copy link
Collaborator Author

There is a problem with download from tarantool.io, so we have 2.x-latest tests failed:

2022-09-12T13:08:26.7307211Z ##[group]Run curl -L https://tarantool.io/pre-release/2/installer.sh | sudo bash
2022-09-12T13:08:26.7307719Z �[36;1mcurl -L https://tarantool.io/pre-release/2/installer.sh | sudo bash�[0m
2022-09-12T13:08:26.7308061Z �[36;1msudo apt install -y tarantool tarantool-dev�[0m
2022-09-12T13:08:26.7365945Z shell: /usr/bin/bash -e {0}
2022-09-12T13:08:26.7366196Z ##[endgroup]
2022-09-12T13:08:26.7606388Z   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
2022-09-12T13:08:26.7608586Z                                  Dload  Upload   Total   Spent    Left  Speed
2022-09-12T13:08:26.7608776Z 
2022-09-12T13:08:27.3371045Z   0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
2022-09-12T13:08:27.3414516Z   0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
2022-09-12T13:08:27.3415325Z   0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
2022-09-12T13:08:27.3420023Z curl: (60) SSL certificate problem: certificate has expired
2022-09-12T13:08:27.3420600Z More details here: https://curl.haxx.se/docs/sslcerts.html

I think we can ignore this problem for a couple of days.

@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-176-queue-loves-pool branch from 679ab77 to 70ad414 Compare September 12, 2022 13:21
@oleg-jukovec oleg-jukovec marked this pull request as ready for review September 12, 2022 13:36
Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

Thank you for your patch!

I have lest several questions. Most of them are not of a great importance, except for the lack of test for the main feature. (Maybe I had missed something.)

CHANGELOG.md Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
connection_pool/connection_pool.go Show resolved Hide resolved
connection_pool/connector_test.go Show resolved Hide resolved
connection_pool/connector_test.go Show resolved Hide resolved
connection_pool/connection_pool.go Show resolved Hide resolved
queue/queue_test.go Show resolved Hide resolved
queue/example_connection_pool_test.go Outdated Show resolved Hide resolved
queue/example_connection_pool_test.go Show resolved Hide resolved
queue/example_connection_pool_test.go Outdated Show resolved Hide resolved
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-176-queue-loves-pool branch from 70ad414 to d0314fe Compare September 13, 2022 14:59
@oleg-jukovec oleg-jukovec requested review from vr009 and removed request for AnaNek September 15, 2022 10:12
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-176-queue-loves-pool branch from d0314fe to 5bc9814 Compare September 16, 2022 10:55
Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

Sorry, it doesn't seems nice to remove the approval (especially since nothing had changed since last revision), but I've tried to manually run the examples and it wasn't an immediate success.

queue/example_connection_pool_test.go Outdated Show resolved Hide resolved
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-176-queue-loves-pool branch 2 times, most recently from ac49d45 to ef7fbd3 Compare September 20, 2022 13:46
The patch adds missed Execute, ExecuteTyped and ExecuteAsync methods
to the ConnectionPool type.

Part of #176
ConnectorAdapter allows to use ConnectionPool as Connector. All
requests to a pool will be executed in a specified mode.

Part of #176
It fixes queue.cfg({in_replicaset = true}) for Tarantool 1.10 [1].

1. tarantool/queue#185

Part of #176
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-176-queue-loves-pool branch 3 times, most recently from 1c6c78a to 3ddf87d Compare September 22, 2022 15:44
@oleg-jukovec
Copy link
Collaborator Author

Sorry, it doesn't seems nice to remove the approval (especially since nothing had changed since last revision), but I've tried to manually run the examples and it wasn't an immediate success.

I have updated the example connection_pool + queue. Let's have another round of review.

@DifferentialOrange DifferentialOrange deleted the oleg-jukovec/gh-176-queue-loves-pool branch September 23, 2022 07:30
@DifferentialOrange DifferentialOrange restored the oleg-jukovec/gh-176-queue-loves-pool branch September 23, 2022 07:34
@DifferentialOrange
Copy link
Member

Sorry, I don't get how force-pulling the branch had resulted in its deleting on the remote.

@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-176-queue-loves-pool branch 3 times, most recently from 917b6ad to 0132b18 Compare September 23, 2022 08:42
The example demonstrates how to use the queue package with
the connection_pool package.

Closes #176
@oleg-jukovec oleg-jukovec force-pushed the oleg-jukovec/gh-176-queue-loves-pool branch from 0132b18 to 842866f Compare September 23, 2022 08:44
Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

Thank you, seems awesome.

I've also tried to ack the taken task after one more master switch, it was a success. If you want, you may add it too, see git diff below.

diff --git a/queue/example_connection_pool_test.go b/queue/example_connection_pool_test.go
index bf1d53d..7b4deae 100644
--- a/queue/example_connection_pool_test.go
+++ b/queue/example_connection_pool_test.go
@@ -185,17 +185,50 @@ func Example_connectionPool() {
        }
 
        // Take a data from the new master instance.
-       if task, err := q.Take(); err == nil || task.Data() == nil {
-               task.Ack()
+       var task *queue.Task
+       task, err = q.Take()
+       if err == nil || task.Data() == nil {
                fmt.Println("Got data:", task.Data())
        } else {
                fmt.Println("Unable to got data:", err)
        }
 
+       // Switch a master instance in the pool.
+       roles = []bool{false, true}
+       err = test_helpers.SetClusterRO(servers, connOpts, roles)
+       if err != nil {
+               fmt.Printf("Unable to set cluster roles: %s", err)
+               return
+       }
+
+       // Wait for a new master instance re-identification.
+       <-h.masterUpdated
+       if h.err != nil {
+               fmt.Printf("Unable to re-identify in the pool: %s", h.err)
+               return
+       }
+
+       err = task.Ack()
+
+       if err != nil {
+               fmt.Printf("Unable to ack the task: %s", err)
+               return
+       }
+
+       var stats interface{}
+       stats, err = q.Statistic()
+       if err != nil {
+               fmt.Printf("Unable to get stats: %s", err)
+               return
+       }
+       fmt.Println(stats)
+
        // Output:
        // Master 127.0.0.1:3014 is ready to work!
        // A Queue object is ready to work.
        // Send data: test_data
        // Master 127.0.0.1:3014 is ready to work!
        // A Queue object is ready to work.
        // Send data: test_data
        // Master 127.0.0.1:3015 is ready to work!
        // Got data: test_data
+       // Master 127.0.0.1:3014 is ready to work!
+       // map[calls:map[ack:1 bury:0 delay:0 delete:0 kick:0 put:1 release:0 take:0 touch:0 ttl:0 ttr:0] tasks:map[buried:0 delayed:0 done:1 ready:0 taken:0 total:0]]
 }

@oleg-jukovec oleg-jukovec requested review from vr009 and removed request for vr009 September 23, 2022 10:33
Copy link

@vr009 vr009 left a comment

Choose a reason for hiding this comment

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

LGTM

@oleg-jukovec oleg-jukovec merged commit 2603eda into master Sep 30, 2022
@oleg-jukovec oleg-jukovec deleted the oleg-jukovec/gh-176-queue-loves-pool branch September 30, 2022 10:32
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.

Make queue works with connection_pool
3 participants