-
Notifications
You must be signed in to change notification settings - Fork 174
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
SUP-20262: increase process timeout to 20 min for entries > 2GB #9131
Conversation
@@ -265,6 +269,7 @@ protected function doSubmit(KalturaDistributionSubmitJobData $data, KalturaYoutu | |||
$media->setFileSize(filesize($videoPath)); | |||
$ingestedVideo = self::uploadInChunks($media,$videoPath, self::DEFAULT_CHUNK_SIZE_BYTE); | |||
$client->setDefer(false); | |||
$this->shouldIncreaseProcessedTimeout(filesize($videoPath)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You call this function should which usually means it should return true or false but the function actually does change the timeout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to "setLongProcessedTimeout($fileSize);"
But it doesn't always 'set' it (depends on fileSize)
If you have a better name suggestions - I'm open to it.
@@ -41,6 +41,10 @@ class YoutubeApiDistributionEngine extends DistributionEngine implements | |||
|
|||
const TIME_TO_WAIT_FOR_YOUTUBE_TRANSCODING = 5; | |||
|
|||
const SIZE_2_GB = 2147483648; | |||
|
|||
const LONG_PROCESS_TIMEOUT = 1200; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it better to change it to a configuration value instead of const in the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to variables
{ | ||
if (isset(KBatchBase::$taskConfig->params->youtubeApi)) | ||
{ | ||
if (isset(KBatchBase::$taskConfig->params->youtubeApi->longProcessedTimeout)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use {} for every if
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
if (isset(KBatchBase::$taskConfig->params->youtubeApi)) | ||
{ | ||
if (isset(KBatchBase::$taskConfig->params->youtubeApi->longProcessedTimeout)) | ||
$this->longProcessedTimeout = KBatchBase::$taskConfig->params->youtubeApi->longProcessedTimeout; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If value is not set in the configuration file - supply default value hard coded
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default values set lines 33-35
if (isset(KBatchBase::$taskConfig->params->youtubeApi->longProcessedTimeout)) | ||
$this->longProcessedTimeout = KBatchBase::$taskConfig->params->youtubeApi->longProcessedTimeout; | ||
|
||
if (isset(KBatchBase::$taskConfig->params->youtubeApi->bigFile)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here - if no value , then set default hard coded.
if ($fileSize > $this->bigFile) | ||
{ | ||
$this->processedTimeout = $this->longProcessedTimeout; | ||
KalturaLog::info('Increased processed timeout to '.$this->processedTimeout.' seconds for file larger than 2GB'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change the message - larger then $filesize
protected $processedTimeout = 300; | ||
protected $longProcessedTimeout = 1200; | ||
protected $bigFile = 2147483648; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MosheMaorKaltura default values
@@ -30,7 +30,9 @@ class YoutubeApiDistributionEngine extends DistributionEngine implements | |||
{ | |||
protected $tempXmlPath; | |||
protected $timeout = 90; | |||
protected $processedTimeout = 300; | |||
protected $processedTimeout = 300; | |||
protected $longProcessedTimeout = 1200; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was to use this default values if they are not set in configuration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I set both properties on the class, then we either override them from config (if config exist) otherwise we keep the default values defined on class.
protected $processedTimeout = 300;
protected $longProcessedTimeout = 1200;
Let me know if I confused it.
reviewed |
No description provided.