Skip to content
This repository was archived by the owner on May 10, 2018. It is now read-only.

Timing Issue in the Splunk_Job class #34

Open
narkaTee opened this issue Jul 8, 2015 · 6 comments
Open

Timing Issue in the Splunk_Job class #34

narkaTee opened this issue Jul 8, 2015 · 6 comments

Comments

@narkaTee
Copy link

narkaTee commented Jul 8, 2015

Hi,

I've found an issue in the fetch method of the Splunk_Job class.

If you try to run a complex search query and/or the searchhead is under heavy load a query can stay in the Queued or Parsing dispatchState for longer than ~1 second which leads to an Splunk_HttpException with the Error Message "HTTP 200 OK".

The Problem occurs if you run the search async like in this example.
The method calls isDone, getProgress and refresh are possible triggers as well as any other method invoking a method which calls Splunk_Job->fetch.

There are multiple ways (increase timeout, increase number of retries...) how to work around this issue but to properly resolve it a proper isReady method should be implemented and used.

For example see how the python sdk implemented this:
is_ready method
usage of this method (just ignore the missing call to sleep before pass command ;))

It's an elegant solution. Whats your opinion?

@shakeelmohamed
Copy link
Contributor

Hi @narkaTee,

Updating Job::isReady() to invoke refresh() seems like a reasonable change.
We should use the logic from the Java SDK where we only refresh() if !Jobs->isReady().

Would you be willing to submit a pull request against the develop branch?

@narkaTee
Copy link
Author

Hi @shakeelmohamed,

I'll look into this when I find some free time. But the current refresh method relies on the Splunk_Job->fetch method as well. This will require some larger changes.

@narkaTee
Copy link
Author

Hi @shakeelmohamed,

I have a version which completes all test except one.
The JobsTest::testItems test Fails:

1) JobsTest::testItems
Splunk_HttpException: HTTP 404 Not Found -- Unknown sid.

/root/dev/public-splunk-php-sdk/Splunk/Http.php:149
/root/dev/public-splunk-php-sdk/Splunk/Http.php:63
/root/dev/public-splunk-php-sdk/Splunk/Http.php:32
/root/dev/public-splunk-php-sdk/Splunk/Context.php:167
/root/dev/public-splunk-php-sdk/Splunk/Context.php:118
/root/dev/public-splunk-php-sdk/Splunk/Endpoint.php:109
/root/dev/public-splunk-php-sdk/Splunk/Endpoint.php:64
/root/dev/public-splunk-php-sdk/Splunk/Entity.php:129
/root/dev/public-splunk-php-sdk/Splunk/Job.php:55
/root/dev/public-splunk-php-sdk/Splunk/Job.php:86
/root/dev/public-splunk-php-sdk/Splunk/Job.php:126
/root/dev/public-splunk-php-sdk/tests/JobsTest.php:107

For some reason the Splunk_Collection::items method returns a stale job which results in the $curJob->getName() call triggering the 404 Error.

I tracked the problem down but I don't know how to solve it. What happens is:
$curJob->getName() issues a call to isReady() which tries to hits the API with:
https://192.168.12.11:8089/servicesNS/admin/search/search/jobs/search+index%3D_internal+%7C+head+10+%7C+top+sourcetype which results in the 404 Error.

The Splunk_Entity::$path is set to search/search/jobs/search+index%3D_internal+%7C+head+10+%7C+top+sourcetype by the Splunk_Collection::loadEntityFromEntry() method via $this->getEntityPath($entry->title).

Is there a reason why the title is used for the path?

I've build a "workaround" in the bug/collectionlist branch.
Does it make sense? Or did I miss something?

@shakeelmohamed
Copy link
Contributor

Thanks for investigating this @narkaTee!

I looked at your fix (https://github.com/narkaTee/splunk-sdk-php/commit/289720cbfcfe499dce487dd29c8882f1fb3dafad) for this specific issue, which seems reasonable.

I think this fix https://github.com/narkaTee/splunk-sdk-php/commit/16415aaff59d6f94dbf4540f1b0262548fda79ff is not what we want. Not every Entity will have a value sid since that's a unique property to Jobs. Can you try moving your modified loadEntityFromEntry() function into Job.php? This other fix https://github.com/narkaTee/splunk-sdk-php/commit/2fb6301a61e656753a14c2e123705e110e449cc9 should be moved into Jobs.php. Overriding those functions in Job.php and Jobs.php should just work because of the inheritance.

@narkaTee
Copy link
Author

narkaTee commented Aug 6, 2015

Hi @shakeelmohamed

ah okay I totally overlooked the Splunk_Jobs Class. Probably due to a lack of coffee ;)

Can you try moving your modified loadEntityFromEntry() function into Job.php?

Did you mean Jobs.php? The only usage of the loadEntityFromEntry Method is in Splunk_Collection::loadEntitiesFromResponse?

I moved the modified loadEntityFromEntry() method to Jobs.php, see fe2384a.
The Tests are all passing with this fix.
I'm currently assuming a Job must have a SID, is that correct? or should I implement a fallback to the title like in 2fb6301?

@shakeelmohamed
Copy link
Contributor

I think 2fb6301 is not really solving the problem, but fe2384a seems like a good solution. Jobs will have a SID unless they're run with oneshot or blocking mode (see the REST API docs for more details).

When you're ready, open a pull request against the develop branch. It will be easier to review your changes that way instead of commit by commit 😄

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

No branches or pull requests

2 participants