Add possibility to read input files in parallel

[ylegat]

Hey !
My team is currently playing with Gor to see if it fits our needs. To give you more context, we have an application deployed on 2 servers in production and we want to monitor their I/O so we can validate the next version of the application (using the middleware feature).

The idea is to write those I/O inside a file, and then replay them on the application to validate. The problem is that, as we have two servers, we will have two log files. If I understand the code correctly, the current behaviour of Gor is to sort input files matching a pattern and read them in order. What we need is to read all input files "in parallel" so we can respect timestamp while replaying requests. We would compare next request timestamps to execute for each file, replay the oldest one, and loop until all files have been read.

Do you think it would be possible to add some kind of flag as a Gor parameter, so that requests coming from input files are executed this way ? If so, my team may help you on that matter and propose a Pull Request.

[buger]

Hello! This is a really good point, and there should be a way to handle such cases by default.

What about such logic:

1. We maintain pool of file object and each file will be represented as object { nextTimestamp }
2. For each file, we read the first request and determine its timestamp, and set it as nextTimestamp.
3. When we need to emit next requests, we go through the list of all potential files and choose one with the earliest nextTimestamp.
4. When we read request from the file, it updates nextTimestamp value with next requests meta. GOTO 3.

What do you think?

[ylegat]

Hello @buger, this is indeed what I had in mind.
When you say a way to handle such cases by default, do you mean that you would like this logic to replace the current one ?

[buger]

@ylegat yes, current logic is just very simplified variant of solution proposed here, and it should be replaced.

[buger]

@ylegat I would love to include this feature in the next release and wanted to ask if you still want to implement this feature, or I can do it with your with your guidance/testing?

Thank you!

[ylegat]

@buger , actually we have already started to implement the feature. Hopefully we will be able to submit a pull request in the course of the week ;)

[ylegat]

Hey @buger , we are currently fighting with a concurrency issue that, I think, is already present in the code. The issue is related to FileInput.Close() method which closes FileInput.currentFile file. This method is not called inside a critical section (using mutex for instance) and the file is also manipulated by the go routine (go i.emit()) so I think this can lead to a data race.

The problem does not seem to occur with the existing code, but it appears that our refactoring has waking him up and TestInputFileLoop is now failing randomly...

There are two solutions for this:

1. manipulate files using a mutex, but this could degrade the performance of the application
2. just get rid of the Close() method, after all it is used only once, in one test, and is not even necessary with the our refactoring

What do you propose ?

[ylegat]

Ok I spoke too quickly regarding InputFile.Close() : our refactoring does not apply that the files will be closed with test TestInputFileLoop. But the data race remains, so we propose instead to use a finalizer (see here)

[ylegat]

EDIT I just tried with v0.14.1, it is ok now

Another thing, I'm using Go 1.6 to build the project and, even so the build is ok, I've got an error when I launch the app :

$ sudo ./gor --input-raw :8086           \ 
             --input-raw-track-response  \
             --output-file ./request.gor

panic: reflect: Call using main.FileOutputConfig as type *main.FileOutputConfig

goroutine 1 [running]:
panic(0x7ea5a0, 0xc821344200)
    /opt/go/src/runtime/panic.go:464 +0x3e6
reflect.Value.call(0x86f980, 0xa5d4e0, 0x13, 0x960100, 0x4, 0xc82130c150, 0x2, 0x2, 0x0, 0x0, ...)
    /opt/go/src/reflect/value.go:372 +0x74a
reflect.Value.Call(0x86f980, 0xa5d4e0, 0x13, 0xc82130c150, 0x2, 0x2, 0x0, 0x0, 0x0)
    /opt/go/src/reflect/value.go:303 +0xb1
main.registerPlugin(0x86f980, 0xa5d4e0, 0xc821151cd0, 0x2, 0x2)
    /home/yohan/go/src/github.com/ylegat/gor/plugins.go:57 +0x396
main.InitPlugins()
    /home/yohan/go/src/github.com/ylegat/gor/plugins.go:120 +0xe51
main.main()
    /home/yohan/go/src/github.com/ylegat/gor/gor.go:60 +0xb95

I tested it with branches master and tag v0.14.0 with the same result. But it works with tag v0.13.0. Any idea about the reason ?

[buger]

I added few mutexes which fix file variable data races, and it does not affect performance. There is still races related to filename but they are safe, its ok to have race there.

And as you mentioned 0.14.1 fixes --output-file issue.

[ylegat]

I added few mutexes which fix file variable data races, and it does not affect performance. There is still races related to filename but they are safe, its ok to have race there.

Sorry I may have not been clear enough: I was actually talking about input_file.go and its Close method. We can use a mutex also there, or use a finalizer, as you wish !

[buger]

Oh, yes, i misread your message. Using finalizer is definitely too hacky, we need to find mutex way.

It can be done similar to output_file change, only when err == io.EOF

[ylegat]

Hey @buger, I think we have a better way than using a mutex : we can achieve the same result with non blocking channel ! But we have a remaining question about this line. Why do you check len(meta) > 2 ? Is it possible than the meta does not contains all triplet (input_type, request_id, timestamp) ?

[buger]

I'll be frank I do not remember why it was added, and for me right now it make no sense. I think it is safe to remove it :)

[ylegat]

Done, Pull-Request ready for review !