vkuznet / transfer2go

Distributed, loosely couple agent-based transferring system
MIT License
8 stars 2 forks source link

Switch to pull model #26

Closed rishiloyola closed 7 years ago

rishiloyola commented 7 years ago

Note: Do not merge this request. @vkuznet I created this request so that you can review and track my work.

vkuznet commented 7 years ago

and, it will create conflict, that's why I suggested to rebase.

On 0, Rishi notifications@github.com wrote:

rishiloyola commented on this pull request.

@@ -1,16 +1,18 @@ package core

// transfer2go core data transfer module -// Copyright (c) 2017 - Valentin Kuznetsov vkuznet@gmail.com +// Author - Valentin Kuznetsov vkuznet@gmail.com

I changed it before you made changes.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/vkuznet/transfer2go/pull/26#discussion_r121457822

rishiloyola commented 7 years ago

Roadmap:

vkuznet commented 7 years ago

You should always assume that requests will come in bulk. But once they arrived you should treat them asynchronously. As I implemented in push model you need to have a job pool. This pool will take requests and perform an action. For instance, the transfer is not an instance operation, especially if we need to transfer GBytes of data. And, since we should develop code which can do a transfer or use external tool, we need further abstraction middleware layer.

What would happen that you'll rely on database to record the action progress. For instance, when your job pool consume new request, it will write to DB, started status. Then it will delegate request to middleware for execution. Periodically it may query this middleware about transfer status and update DB about it. Once done it will record in DB that transfer is finished and remove request from the pool.

On 0, Rishi notifications@github.com wrote:

rishiloyola commented on this pull request.

@@ -134,8 +134,9 @@ func AuthHandler(w http.ResponseWriter, r *http.Request) { case "verbose": VerboseHandler(w, r) case "list":

  • // TODO: check whether parameter is there or not....
  • ListHandler(w, r, strings.Split(r.URL.RawQuery, "=")[1])
  • ListHandler(w, r, r.URL.RawQuery)
  • case "action":
  • ActionHandler(w, r)

@vkuznet I have added /action endpoint to perform actions on requests. It can also support bulk operations. Test Command :

curl -X POST http://localhost:8989/action -d '[{"id":"123","action":"delete"},{"id":"789","action":"transfer"}]' My question is: How do you want to handle this load? There can be single operation or bulk operations.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/vkuznet/transfer2go/pull/26#pullrequestreview-43696128

rishiloyola commented 7 years ago

So our system will have two kinds of job pools? One is to handle the incoming requests and another one is to transfer requests.

vkuznet commented 7 years ago

It seems logical, doesn't it?

On 0, Rishi notifications@github.com wrote:

So our system will have two kinds of job pools? One is to handle the incoming requests and another one is to transfer requests.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/vkuznet/transfer2go/pull/26#issuecomment-308477880

rishiloyola commented 7 years ago

@vkuznet Can you tell me why are you creating this file? Code I am planning to use transfer pull as an instance of Dispatcher.

vkuznet commented 7 years ago

it is a metrics log file. It should hold all server metrics. If number metrics grow or their frequency it is better to keep them in separate file instead of server log.

On 0, Rishi notifications@github.com wrote:

@vkuznet Can you tell me why are you creating this file? Code I am planning to use transfer pull as an instance of Dispatcher.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/vkuznet/transfer2go/pull/26#issuecomment-308821107

rishiloyola commented 7 years ago

@vkuznet Can you review my last commit?

New Changes:

rishiloyola commented 7 years ago

Sure, I will make them as a separate db while implementing generic database wrapper.

rishiloyola commented 7 years ago

@vkuznet can you review this commit? I made separate folders to save requests and dataset related queries. commit

rishiloyola commented 7 years ago

TODOs:

rishiloyola commented 7 years ago

HTML interface: screen shot 2017-07-08 at 2 04 54 am

vkuznet commented 7 years ago

than it's ok, we don't need extra unit test

On 0, Rishi notifications@github.com wrote:

rishiloyola commented on this pull request.

@@ -0,0 +1,55 @@ +package core

It's standard Go library. They already wrote unit tests for this package. code

Do you want to add that test file in our project?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/vkuznet/transfer2go/pull/26#discussion_r126191255

vkuznet commented 7 years ago

Rishi, the web interface will need to be adjusted since we gonna have long names. Here is a typical name of the block: /ZMM/Summer11-DESIGN42_V11_428_SLHC1-v1/GEN-SIM#8420a3e7-96cf-48b1-a6a8-f9abcd3de8ef and here one of its file names: /store/mc/Summer11/ZMM/GEN-SIM/DESIGN42_V11_428_SLHC1-v1/0003/02ACAA1A-9F32-E111-BB31-0002C90B743A.root

So I would propose to have instead of tables the list form, e.g.

Request ID: 123456789 Block: /ZMM/Summer11-DESIGN42_V11_428_SLHC1-v1/GEN-SIM#8420a3e7-96cf-48b1-a6a8-f9abcd3de8ef File: /store/mc/Summer11/ZMM/GEN-SIM/DESIGN42_V11_428_SLHC1-v1/0003/02ACAA1A-9F32-E111-BB31-0002C90B743A.root Destination: T1_CH_CERN Source: T1_IT_INFN Status: pending Action: drop-down menu

For instance see this: https://www.dropbox.com/s/sw8p2xfy5mws94l/ReqMgr2.png?dl=0 (it is one of the services I developed for CMS).

But of course, it is not yet relevant until we'll have working functionality.

On 0, Rishi notifications@github.com wrote:

HTML interface: screen shot 2017-07-08 at 2 04 54 am

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/vkuznet/transfer2go/pull/26#issuecomment-313787247

vkuznet commented 7 years ago

good

On 0, Rishi notifications@github.com wrote:

@vkuznet can you review this commit? I made separate folders to save requests and dataset related queries. commit

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/vkuznet/transfer2go/pull/26#issuecomment-313784935

rishiloyola commented 7 years ago

Can you share the code of that HTML page?

Rishi, the web interface will need to be adjusted since we gonna have long names. Here is a typical name of the block: /ZMM/Summer11-DESIGN42_V11_428_SLHC1-v1/GEN-SIM#8420a3e7- 96cf-48b1-a6a8-f9abcd3de8ef and here one of its file names: /store/mc/Summer11/ZMM/GEN-SIM/DESIGN42_V11_428_SLHC1-v1/ 0003/02ACAA1A-9F32-E111-BB31-0002C90B743A.root

So I would propose to have instead of tables the list form, e.g.

Request ID: 123456789 Block: /ZMM/Summer11-DESIGN42_V11_428_SLHC1-v1/GEN-SIM#8420a3e7- 96cf-48b1-a6a8-f9abcd3de8ef File: /store/mc/Summer11/ZMM/GEN-SIM/DESIGN42_V11_428_SLHC1-v1/ 0003/02ACAA1A-9F32-E111-BB31-0002C90B743A.root Destination: T1_CH_CERN Source: T1_IT_INFN Status: pending Action: drop-down menu

For instance see this: https://www.dropbox.com/s/ sw8p2xfy5mws94l/ReqMgr2.png?dl=0 (it is one of the services I developed for CMS).

But of course, it is not yet relevant until we'll have working functionality.

On 0, Rishi notifications@github.com wrote:

HTML interface: screen shot 2017-07-08 at 2 04 54 am

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/vkuznet/transfer2go/pull/26#issuecomment-313787247

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/vkuznet/transfer2go/pull/26#issuecomment-313870922, or mute the thread https://github.com/notifications/unsubscribe-auth/AJoIV561rQ-nIY2q-anOVMCRBOMpsYzaks5sL8GbgaJpZM4NyeTs .

vkuznet commented 7 years ago

The code comes from CMS ReqMgr data-service (written in python): https://github.com/dmwm/WMCore/blob/master/src/python/WMCore/ReqMgr/Web/ReqMgrService.py#L436 templates, javascript and styles are here: https://github.com/dmwm/WMCore/blob/master/src/html/ReqMgr/jinja_templates/approve.tmpl https://github.com/dmwm/WMCore/blob/master/src/html/ReqMgr/javascript/utils.js https://github.com/dmwm/WMCore/tree/master/src/html/ReqMgr/style

I think it can be easy to adjust since tempalates are written in JINJA which is compatible with Go templates syntax.

On 0, Rishi notifications@github.com wrote:

Can you share the code of that HTML page?

On Sat, Jul 8, 2017 at 11:21 PM, Valentin Kuznetsov < notifications@github.com> wrote:

Rishi, the web interface will need to be adjusted since we gonna have long names. Here is a typical name of the block: /ZMM/Summer11-DESIGN42_V11_428_SLHC1-v1/GEN-SIM#8420a3e7- 96cf-48b1-a6a8-f9abcd3de8ef and here one of its file names: /store/mc/Summer11/ZMM/GEN-SIM/DESIGN42_V11_428_SLHC1-v1/ 0003/02ACAA1A-9F32-E111-BB31-0002C90B743A.root

So I would propose to have instead of tables the list form, e.g.

Request ID: 123456789 Block: /ZMM/Summer11-DESIGN42_V11_428_SLHC1-v1/GEN-SIM#8420a3e7- 96cf-48b1-a6a8-f9abcd3de8ef File: /store/mc/Summer11/ZMM/GEN-SIM/DESIGN42_V11_428_SLHC1-v1/ 0003/02ACAA1A-9F32-E111-BB31-0002C90B743A.root Destination: T1_CH_CERN Source: T1_IT_INFN Status: pending Action: drop-down menu

For instance see this: https://www.dropbox.com/s/ sw8p2xfy5mws94l/ReqMgr2.png?dl=0 (it is one of the services I developed for CMS).

But of course, it is not yet relevant until we'll have working functionality.

On 0, Rishi notifications@github.com wrote:

HTML interface: screen shot 2017-07-08 at 2 04 54 am

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/vkuznet/transfer2go/pull/26#issuecomment-313787247

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/vkuznet/transfer2go/pull/26#issuecomment-313870922, or mute the thread https://github.com/notifications/unsubscribe-auth/AJoIV561rQ-nIY2q-anOVMCRBOMpsYzaks5sL8GbgaJpZM4NyeTs .

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/vkuznet/transfer2go/pull/26#issuecomment-313902191

rishiloyola commented 7 years ago

@vkuznet I added separate DB to store data and requests. Can you review this commit? commit I designed it in such a way that in future we can use different database type for different purposes.

vkuznet commented 7 years ago

Rishi, why do we need separate DB? why not to use separate DB tables within the same DB? If we're going to use SQLite, separate DB means separate file, while using MySQL/Oracle/Non-SQLite DB separate DB means different host, port, namespace.

I don't see the point (yet) of having separate DB, but it complicates code and configuration. For instance you have separate DB name, then you have separate TFC namespace, etc.

Please justify the requirements to have separate DB. I'll suggest to keep code around if we need such a change. Plus, I don't really want to put many different changes to a single PR. It is already big enough that introducing new feature complicate it a lot. Let's finish the PR about the push/pull model and commit. Then have separate PR with changes (if necessary) for DB splitting.

Valentin.

On 0, Rishi notifications@github.com wrote:

@vkuznet I added separate db to store data and requests. Can you review this commit? commit I implemented in such a way that in future we can use different database type for different purposes.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/vkuznet/transfer2go/pull/26#issuecomment-314168879

rishiloyola commented 7 years ago

I guess we discussed on phone to make separate databases to store requests and data.

rishiloyola commented 7 years ago

Okay, I reset my last commit. You can merge this request.

vkuznet commented 7 years ago

Sorry, probably my fault. We can still use the same physical database but use different namespace and/or tables. We can discuss it later.

On 0, Rishi notifications@github.com wrote:

I guess we discussed on phone to make separate databases to store requests and data.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/vkuznet/transfer2go/pull/26#issuecomment-314182499

vkuznet commented 7 years ago

Rishi, I'll review it one more time tomorrow and merge. Tonight I have other task to take care of.

Let's chat may be tomorrow/Wed.

Best, Valentin.

On 0, Rishi notifications@github.com wrote:

Okay, I reset my last commit. You can merge this request.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/vkuznet/transfer2go/pull/26#issuecomment-314182985

rishiloyola commented 7 years ago

@vkuznet I have added complete test suite. If you want to test it then just run this command

bash test.sh

it will automatically start three agents(main, source, destination) and will test all the endpoints.