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

Add counts and add_new_item methods, without tests #4

Merged
merged 47 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
1260925
Implemented no duplicate when adding items and tests, atomic length.
Oxygen588 Jun 30, 2023
f4bd9c2
Remove `tests/go/go` binary
JOT85 Jul 3, 2023
28cd1c9
Merge branch 'JavaScript-Implementation' into Atomic-length-method
JOT85 Jul 3, 2023
b101df6
Update WorkQueue.ts
Oxygen588 Jul 4, 2023
19989b1
Atomic functions fix.
Oxygen588 Jul 4, 2023
50069e9
Update index.ts
Oxygen588 Jul 4, 2023
8abf3ed
Update run-test.sh
Oxygen588 Jul 4, 2023
1aa44b3
Atomic lengths Python Fix.
Oxygen588 Jul 4, 2023
61ac4a2
Rust-lang Atomicity functions fix
Oxygen588 Jul 4, 2023
67b5c70
Update main.rs
Oxygen588 Jul 4, 2023
0255224
Update WorkQueue.go
Oxygen588 Jul 4, 2023
1d8fe42
Update WorkQueue.go
Oxygen588 Jul 4, 2023
dc99a48
Update WorkQueue.go
Oxygen588 Jul 4, 2023
08c47ca
Atomicity fully fixed + tests fixed.
Oxygen588 Jul 4, 2023
5bf622e
Update WorkQueue.go
Oxygen588 Jul 4, 2023
d139f55
Update WorkQueue.ts
Oxygen588 Jul 4, 2023
7c5fc9b
AddAtomicItem Rename + Better Doc Comment
Oxygen588 Jul 5, 2023
e6db6b1
Update WorkQueue.go
Oxygen588 Jul 5, 2023
0dfd47a
AddItemAtomically, Lengths small changes
Oxygen588 Jul 5, 2023
292f428
Update WorkQueue.ts
Oxygen588 Jul 5, 2023
2529278
Fixed add_item_atomically.
Oxygen588 Jul 5, 2023
f07d603
Unexecuted pipe fix Ts, necessary fixes done within addAtomicItem Ts,…
Oxygen588 Jul 5, 2023
d804ee0
Update workqueue.py
Oxygen588 Jul 5, 2023
89e156c
Update WorkQueue.go
Oxygen588 Jul 5, 2023
1cd515e
Update WorkQueue.ts
Oxygen588 Jul 5, 2023
b34629a
Necessary Fixes + Documentation.
Oxygen588 Jul 5, 2023
c3c40f5
Docs for Length function
Oxygen588 Jul 5, 2023
29b4235
Renamed functions from AddAtomicItem to AddNewItem as well as removed…
Oxygen588 Sep 25, 2023
8523175
Delete redis-work-queue.sln
Oxygen588 Sep 25, 2023
61a5052
Necessary fixes.
Oxygen588 Sep 26, 2023
612b776
Update docs, rename `Lengths` -> `Counts` and small tweaks to impleme…
JOT85 Sep 26, 2023
0212ee8
Merge branch 'Atomic-length-method' of github.com:Oxygen588/redis-wor…
JOT85 Sep 26, 2023
14d9392
Fixes.
Oxygen588 Sep 26, 2023
6de4de9
Fixed addNewItem & tests for it.
Oxygen588 Sep 28, 2023
522c5a6
Fixes.
Oxygen588 Sep 29, 2023
ccb6f6e
rust: format and remove unused dependencies and imports
JOT85 Oct 5, 2023
04dbdbd
go: AddNewItem: correct TxPipeline usage, correct error message, form…
JOT85 Oct 5, 2023
a953fce
go: AddNewItem: remove retry limit
JOT85 Oct 5, 2023
9b4dfc5
go: Counts: format
JOT85 Oct 5, 2023
07e11a3
python: format
JOT85 Oct 5, 2023
37478c6
node: AddNewItem: correct retry logic and remove logs
JOT85 Oct 5, 2023
5cf7f41
node: format
JOT85 Oct 5, 2023
6d41f55
rust: rename `get_queue_lengths` -> `counts` and add docs
JOT85 Oct 5, 2023
4f5349d
Small fixes
Oxygen588 Oct 11, 2023
2b81138
Restore tests to upstream
JOT85 Oct 27, 2023
7a7294b
Readd tests/node/package-lock.json
JOT85 Oct 27, 2023
994b09f
go: correctly return boolean indicating if an item was added in AddNe…
JOT85 Oct 27, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 111 additions & 1 deletion go/WorkQueue.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ package workqueue

import (
"context"
"errors"
"time"

"github.com/google/uuid"
Expand Down Expand Up @@ -120,8 +121,117 @@ func (workQueue *WorkQueue) AddItem(ctx context.Context, db *redis.Client, item
return err
}

// AddNewItem adds an item to the work queue only if an item with the same ID doesn't already exist.
//
// This method uses WATCH to add the item atomically. The db client passed must not be used by
// anything else while this method is running.
//
// Returns a boolean indicating if the item was added or not. The item is only not added if it
// already exists or if an error (other than a transaction error, which triggers a retry) occurs.
func (workQueue *WorkQueue) AddNewItem(ctx context.Context, db *redis.Client, item Item) (bool, error) {
txf := func(tx *redis.Tx) error {

processingItemsInQueueCmd := tx.LPos(ctx, workQueue.processingKey, item.ID, redis.LPosArgs{
Rank: 0,
MaxLen: 0,
})
workingItemsInQueueCmd := tx.LPos(ctx, workQueue.mainQueueKey, item.ID, redis.LPosArgs{
Rank: 0,
MaxLen: 0,
})

_, ProcessingQueueCheck := processingItemsInQueueCmd.Result()
_, WorkingQueueCheck := workingItemsInQueueCmd.Result()

if ProcessingQueueCheck == redis.Nil && WorkingQueueCheck == redis.Nil {

_, err := tx.TxPipelined(ctx, func(pipe redis.Pipeliner) error {
workQueue.AddItemToPipeline(ctx, pipe, item)
_, err := pipe.Exec(ctx)
return err
})
return err
} else {
return nil
}
}

for i := 0; i < 100; i++ {
err := db.Watch(ctx, txf, workQueue.processingKey, workQueue.mainQueueKey)
if err == nil {
return true, nil
}
if err == redis.TxFailedErr {
continue
}
return false, err
}

return false, errors.New("increment reached maximum number of retries")
}

func (workQueue *WorkQueue) AddNewItemWithSleep(ctx context.Context, db *redis.Client, item Item) (bool, error) {
txf := func(tx *redis.Tx) error {

processingItemsInQueueCmd := tx.LPos(ctx, workQueue.processingKey, item.ID, redis.LPosArgs{
Rank: 0,
MaxLen: 0,
})
workingItemsInQueueCmd := tx.LPos(ctx, workQueue.mainQueueKey, item.ID, redis.LPosArgs{
Rank: 0,
MaxLen: 0,
})

_, ProcessingQueueCheck := processingItemsInQueueCmd.Result()
_, WorkingQueueCheck := workingItemsInQueueCmd.Result()

if ProcessingQueueCheck == redis.Nil && WorkingQueueCheck == redis.Nil {
time.Sleep(100 * time.Microsecond)
JOT85 marked this conversation as resolved.
Show resolved Hide resolved
_, err := tx.TxPipelined(ctx, func(pipe redis.Pipeliner) error {
workQueue.AddItemToPipeline(ctx, pipe, item)
_, err := pipe.Exec(ctx)
return err
})
return err
} else {
return nil
}
}

for i := 0; i < 100; i++ {
err := db.Watch(ctx, txf, workQueue.processingKey, workQueue.mainQueueKey)
if err == nil {
return true, nil
}
if err == redis.TxFailedErr {
continue
}
return false, err
}

return false, errors.New("increment reached maximum number of retries")
}
JOT85 marked this conversation as resolved.
Show resolved Hide resolved

// Counts returns the queue length, and number of items currently being processed, atomically.
func (workQueue *WorkQueue) Counts(ctx context.Context, db *redis.Client) (queueLen, processingLen int64, err error) {
tx := db.TxPipeline()

queueLenPipe := tx.LLen(ctx, workQueue.mainQueueKey)
processingLenPipe := tx.LLen(ctx, workQueue.processingKey)

_, err = tx.Exec(ctx)
JOT85 marked this conversation as resolved.
Show resolved Hide resolved
if err == nil {
queueLen, err = queueLenPipe.Result()
}
if err == nil {
processingLen, err = processingLenPipe.Result()
}

return
}

// Return the length of the work queue (not including items being processed, see
// [WorkQueue.Processing]).
// [WorkQueue.Processing] or [WorkQueue.Counts] to get both).
func (workQueue *WorkQueue) QueueLen(ctx context.Context, db *redis.Client) (int64, error) {
return db.LLen(ctx, workQueue.mainQueueKey).Result()
}
Expand Down
143 changes: 127 additions & 16 deletions node/src/WorkQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
* @description A work queue backed by a redis database.
*/

import Redis, {ChainableCommander} from 'ioredis';
import {v4 as uuidv4} from 'uuid';
import {Item} from './Item';
import {KeyPrefix} from './KeyPrefix';
import Redis, { ChainableCommander } from 'ioredis';
import { v4 as uuidv4 } from 'uuid';
import { Item } from './Item';
import { KeyPrefix } from './KeyPrefix';

export {KeyPrefix, Item};
export { KeyPrefix, Item };

/**
* A work queue backed by a redis database.
Expand All @@ -35,10 +35,11 @@ export class WorkQueue {

/**
* Add an item to the work queue. This adds the redis commands onto the pipeline passed.
*
* Use `WorkQueue.addItem` if you don't want to pass a pipeline directly.
* Add the item data.
* @param {Pipeline} pipeline The pipeline that the data will be executed.
* @param {Item} item The Item which will be set in the Redis with the key of this.itemDataKey.of(item.id).
*
* @param {Pipeline} pipeline The pipeline that the commands to add the item will be pushed to.
* @param {Item} item The Item to be added.
*/
addItemToPipeline(pipeline: ChainableCommander, item: Item) {
// NOTE: it's important that the data is added first, otherwise someone before the data is ready.
Expand All @@ -48,11 +49,12 @@ export class WorkQueue {
}

/**
* Add an item to the work queue.
* Add an item to the work queue. See `addNewItem` to avoid adding duplicate items.
*
* This creates a pipeline and executes it on the database.
*
* @param {Redis} db The Redis Connection.
* @param item The item that will be executed using the method addItemToPipeline.
* @param item The item to be added.
*/
async addItem(db: Redis, item: Item): Promise<void> {
const pipeline = db.pipeline();
Expand All @@ -61,7 +63,100 @@ export class WorkQueue {
}

/**
* This is used to get the length of the Main Queue.
* Adds an item to the work queue only if an item with the same ID doesn't already exist.
*
* This method uses WATCH to add the item atomically. The db client passed must not be used by
* anything else while this method is running.
*
* Returns a boolean indicating if the item was added or not. The item is only not added if it
* already exists or if an error (other than a transaction error, which triggers a retry) occurs.
*
* @param {Redis} db The Redis Connection, this must not be used by anything else while this method is running.
* @param item The item that will be added, only if an item doesn't already exist with the same ID.
* @returns {boolean} returns false if already in queue or true if the item is successfully added.
*/
async addNewItem(db: Redis, item: Item): Promise<boolean> {
while (true) {
try {
await db.watch(this.mainQueueKey, this.processingKey);

const isItemInProcessingKey = await db.lpos(this.processingKey, item.id);
const isItemInMainQueueKey = await db.lpos(this.mainQueueKey, item.id);
if (isItemInProcessingKey !== null || isItemInMainQueueKey !== null) {
console.log("Item already exists, not added", item.id);
await db.unwatch();
return false;
}
const transaction = db.multi();
this.addItemToPipeline(transaction, item);
const results = await transaction.exec();

if (!results) {
console.log("Transaction failed, item not added", item.id);
await db.unwatch();
}
console.log("Item added successfully", item.id);
return true
} catch (e) {
console.log("Error", e);
} finally {
await db.unwatch();
}
}
return false;
}

/**
* This is a dummy function used for testing!
*
* Adds an item to the work queue only if an item with the same ID doesn't already exist with a 50 ms sleep.
*
* This method uses WATCH to add the item atomically. The db client passed must not be used by
* anything else while this method is running.
*
* Returns a boolean indicating if the item was added or not. The item is only not added if it
* already exists or if an error (other than a transaction error, which triggers a retry) occurs.
*
* @param {Redis} db The Redis Connection, this must not be used by anything else while this method is running.
* @param item The item that will be added, only if an item doesn't already exist with the same ID.
* @returns {boolean} returns false if already in queue or true if the item is successfully added.
*/
async addNewItemWithSeep(db: Redis, item: Item): Promise<boolean> {
while (true) {
try {
await db.watch(this.mainQueueKey, this.processingKey);

const isItemInProcessingKey = await db.lpos(this.processingKey, item.id);
const isItemInMainQueueKey = await db.lpos(this.mainQueueKey, item.id);
if (isItemInProcessingKey !== null || isItemInMainQueueKey !== null) {
console.log("Item already exists, not added", item.id);
await db.unwatch();
return false;
}
await new Promise<void>((resolve) => setTimeout(resolve, 50));;
JOT85 marked this conversation as resolved.
Show resolved Hide resolved
const transaction = db.multi();
this.addItemToPipeline(transaction, item);
const results = await transaction.exec();
Copy link
Member

Choose a reason for hiding this comment

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

@Oxygen588 what does the return value look like in the success and failure case? I'm not sure what it would be, so it would be nice to have a comment!

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, to clarify what I mean, I don't know what the value of results if the transaction succeeds or fails.


if (!results) {
console.log("Transaction failed, item not added", item.id);
await db.unwatch();
}
console.log("Item added successfully", item.id);
return true
} catch (e) {
console.log("Error", e);
} finally {
await db.unwatch();
}
}
return false;
}
JOT85 marked this conversation as resolved.
Show resolved Hide resolved


/**
* Return the length of the work queue (not including items being processed, see
* `WorkQueue.processing` or `WorkQueue.counts` to get both).
*
* @param {Redis} db The Redis Connection.
* @returns {Promise<number>} Return the length of the work queue (not including items being processed, see `WorkQueue.processing()`).
Expand All @@ -71,7 +166,7 @@ export class WorkQueue {
}

/**
* This is used to get the lenght of the Processing Queue.
* This is used to get the number of items currently being processed.
*
* @param {Redis} db The Redis Connection.
* @returns {Promise<number>} The number of items being processed.
Expand All @@ -80,6 +175,22 @@ export class WorkQueue {
return db.llen(this.processingKey);
}

/**
* Returns the queue length, and number of items currently being processed, atomically.
*
* @param {Redis} db The Redis Connection.
* @returns {Promise<[number, number]>} Return the length of main queue and processing queue, respectively.
*/
async counts(db: Redis): Promise<[number, number]> {
const multi = db.multi();
multi.llen(this.mainQueueKey);
multi.llen(this.processingKey);
const result = (await multi.exec()) as Array<[Error | null, number]>;
const queueLength = result[0][1];
const processingLength = result[1][1];
return [queueLength, processingLength];
}

/**
* This method can be used to check if a Lease Exists or not for a itemId.
*
Expand Down Expand Up @@ -116,8 +227,8 @@ export class WorkQueue {
async lease(
db: Redis,
leaseSecs: number,
block = true,
timeout = 1
block: boolean = true,
timeout: number = 1
): Promise<Item | null> {
let maybeItemId: string | null = null;

Expand All @@ -131,7 +242,7 @@ export class WorkQueue {
} else {
maybeItemId = await db.rpoplpush(this.mainQueueKey, this.processingKey);
}

console.log(`Leased ${maybeItemId}`)
JOT85 marked this conversation as resolved.
Show resolved Hide resolved
if (maybeItemId == null) {
return null;
}
Expand Down Expand Up @@ -207,7 +318,7 @@ export class WorkQueue {
if (removed === 0) {
return false;
}

console.log(`Completed ${item.id}`);
JOT85 marked this conversation as resolved.
Show resolved Hide resolved
const pipeline = db.pipeline();
pipeline.del(this.itemDataKey.of(item.id));
pipeline.del(this.leaseKey.of(item.id));
Expand Down
Loading