Skip to content
This repository has been archived by the owner. It is now read-only.

[dev.icinga.com #3325] Oracle: servicestatus/hoststatus queries fail when plugin output is longer than 2048 bytes #1136

Closed
icinga-migration opened this issue Oct 22, 2012 · 15 comments

Comments

Projects
None yet
1 participant
@icinga-migration
Copy link
Member

commented Oct 22, 2012

This issue has been migrated from Redmine: https://dev.icinga.com/issues/3325

Created by gbeutner on 2012-10-22 13:57:15 +00:00

Assignee: Tommi
Status: Resolved (closed on 2012-11-10 15:02:48 +00:00)
Target Version: 1.8.2
Last Update: 2014-12-08 14:37:54 +00:00 (in Redmine)

Icinga Version: 1.10.0
OS Version: any

Example IDO input: https://sbfl.beutner.name/public/gnb/i-hate-oracle

[pid  1906] read(7, "\0\260\0\0\6\0\0\0\0\0\4\1\0\0\0L\0\0\0\0\0X\2\0\0\0\0\t\0\0\0\275\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0N\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0cORA-00600: internal error code, arguments: [koklGetLocAndFlag: bndpos], [], [], [], [], [], [], []\n", 2064) = 176

A quick and dirty fix would be to just truncate the plugin output: https://git.icinga.org/?p=icinga-core.git;a=commit;h=2d7d55cdb4af6b2c535f788993f533c6d3996116

However ideally the output column should probably be changed to CLOB instead. I'm leaving this for someone who is more familiar with Oracle.

Changesets

2012-10-29 18:36:11 +00:00 by mfriedrich b1207bd

update Changelog from previous oracle commits refs #3324 #3325

2012-10-31 20:30:28 +00:00 by Tommi 61a2ecb94ffed571818c6bb11f8ff83b104f7082

idoutils: add fix for output field on statehistory table

refs #3325

Relations:

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 22, 2012

Updated by mfriedrich on 2012-10-22 16:18:16 +00:00

having the fix applied for #3324

Oct 22 18:06:33 sol ido2db: OCIERROR - MSG ORA-01461: can bind a LONG value only for insert into a LONG column#012 at pos 2340 in QUERY 'MERGE INTO servicestatus USING DUAL ON (service_object_id=:X2) WHEN MATCHED THEN UPDATE SET instance_id=:X1, status_update_time=unixts2localts(:X3), output=:X4, long_output=:X5u, perfdata=:X6u, current_state=:X7, has_been_checked=:X8, should_be_scheduled=:X9, current_check_attempt=:X10, max_check_attempts=:X11, last_check=unixts2localts(:X12), next_check=unixts2localts(:X13), check_type=:X14, last_state_change=unixts2localts(:X15), last_hard_state_change=unixts2localts(:X16), last_hard_state=:X17, last_time_ok=unixts2localts(:X18), last_time_warning=unixts2localts(:X19), last_time_unknown=unixts2localts(:X20), last_time_critical=unixts2localts(:X21), state_type=:X22, last_notification=unixts2localts(:X23), next_notification=unixts2localts(:X24), no_more_notifications=:X25, notifications_enabled=:X26, problem_has_been_acknowledged=:X27, acknowledgement_type=:X28, current_notification_number=:X29, passive_checks_enabled=:X30, active_checks_enabled=:X31, event_handler_enabled=:X32, flap_detection_enabled=:X33, is_flapping=:X34, percent_state_change=:X35, latency=:X36, execution_time=:X37, scheduled_downtime_depth=:X38, failure_prediction_enabled=:X39, process_performance_data=:X40, obsess_over_service=:X41, modified_service_attributes=:X42, event_handler=:X43, check_command=:X44, normal_check_interval=:X45, retry_check_interval=:X46, check_timeperiod_object_id=:X47 WHEN NOT MATCHED THEN INSERT (id, instance_id, service_object_id, status_update_time, output, long_output, perfdata, current_state, has_been_checked, should_be_scheduled, current_check_attempt, max_check_attempts, last_check, next_check, check_type, last_state_change, last_hard_state_change, last_hard_state, last_time_ok, last_time_warning, last_time_unknown, last_time_critical, state_type, last_notification, next_notification, no_more_notifications, notifications_enabled, problem_has_been_acknowledged, acknowledgement_t
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 25, 2012

Updated by Tommi on 2012-10-25 12:32:17 +00:00

  • Status changed from New to Assigned
  • Assigned to set to Tommi

will take a look into on Sunday. output column was not intend to hold long and/or binary data. For this we have longoutput column, which is already a clob. Therefore truncating is ok, but looks like we need a binary filter too.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 27, 2012

Updated by Tommi on 2012-10-27 09:25:34 +00:00

none of oracle varchar values are currently checked not exeeding their defined field size. There is currently no mapping code<->sql. Means this issue may happen on every of this columns

action_url varchar2(1024),
  action_url varchar2(1024),
  address6 varchar2(128),
  address varchar2(1024)
  address varchar2(128),
  agent_name varchar2(32),
  agent_version varchar2(16),
  alias varchar2(1024)
  alias varchar2(1024)
  alias varchar2(1024)
  alias varchar2(1024)  
  alias varchar2(1024),
  alias varchar2(64),
  author_name varchar2(64),
  author_name varchar2(64),
  author_name varchar2(64),
  author_name varchar2(64),
  author_name varchar2(64),
  check_command_args varchar2(1024),
  check_command_args varchar2(1024),
  check_command varchar2(1024),
  check_command varchar2(1024),
  command_args varchar2(1024)
  command_args varchar2(1024)
  command_args varchar2(1024)  
  command_args varchar2(1024),
  command_args varchar2(1024),
  command_args varchar2(1024),
  command_line varchar2(2048)
  command_line varchar2(2048),
  command_line varchar2(2048),
  command_line varchar2(2048),
  command_line varchar2(2048),
  command_name varchar2(128),
  comment_data varchar2(2048),
  comment_data varchar2(2048),
  comment_data varchar2(2048),
  comment_data varchar2(2048),
  comment_data varchar2(2048),
  configfile_path varchar2(1024)
  connect_source varchar2(16),
  connect_type varchar2(16),
  display_name varchar2(1024),
  display_name varchar2(1024),
  disposition varchar2(16),
  email_address varchar2(1024),
  eventhandler_command_args varchar2(1024),
  eventhandler_command_args varchar2(1024),
  event_handler varchar2(1024),
  event_handler varchar2(1024),
  failure_prediction_options varchar2(128),
  failure_prediction_options varchar2(64),
  global_host_event_handler varchar2(1024),
  global_service_event_handler varchar2(1024)
  icon_image_alt varchar2(1024)
  icon_image_alt varchar2(1024),
  icon_image varchar2(1024),
  icon_image varchar2(1024),
  instance_description varchar2(128)
  instance_name varchar2(64),
    myfile varchar2(255);
  name1 varchar2(128),
  name2 varchar2(128),
  name varchar2(10),
  notes_url varchar2(1024),
  notes_url varchar2(1024),
  notes varchar2(1024),
  notes varchar2(1024),
  output varchar2(2048),
  output varchar2(2048),
  output varchar2(2048),
  output varchar2(2048),
  output varchar2(2048),
  output varchar2(2048),
  output varchar2(2048),
  output varchar2(2048),
  pager_address varchar2(64),
  program_date varchar2(10)
  program_name varchar2(16),
  program_version varchar2(20),
  statusmap_image varchar2(1024),
    text varchar2(200);
  varname varchar2(1024),
  varname varchar2(1024),
  varname varchar2(64),
  varname varchar2(64),
  varvalue varchar2(1024)
  varvalue varchar2(1024)
  varvalue varchar2(1024)
  varvalue varchar2(2048)
  version varchar2(10),
  vrml_image varchar2(1024),
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 27, 2012

Updated by mfriedrich on 2012-10-27 10:49:36 +00:00

mhm, and the output column is one of those columns which the user (or likely the plugin) can exceed easily. the idea is not to change that for all existing ones, but just in case of failure for that (actually calling check_ssh generating long output, but in the first line - to resolve the issue on the user side you would actually need to rewrite the checkplugin - which is not possible in that case).

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 27, 2012

Updated by Tommi on 2012-10-27 19:37:53 +00:00

a fix for output column isnt enough, atleast command_args and command_line fields needs to be handled additionally and in several tables.
created a fix for this (hope i found all) truncating the data: https://git.icinga.org/?p=icinga-core.git;a=commit;h=995bc8517f7c30338a41798c226811a32c367452.
Testcase for #3324 looks solved in my dev environment, entries appearing in database.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 27, 2012

Updated by Tommi on 2012-10-27 19:38:21 +00:00

  • Status changed from Assigned to Feedback
  • Done % changed from 0 to 90
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 28, 2012

Updated by mfriedrich on 2012-10-28 12:02:22 +00:00

thanks. not really what i was expecting, but if limiting the output is the only solution ...

your branch 'tdressler/fixes' is currently built on top of 'dev/ido' but has 'r1.8' merged - which is an invalid merge source for all dev and next branches. so which commits should be cherry-picked then? 995bc8517f7c30338a41798c226811a32c367452 or more?

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 28, 2012

Updated by Tommi on 2012-10-28 14:35:57 +00:00

All code related to this issue is in this single commit. There is one small change of a constant name in the commit before, which should be taken additionally.

As stated before, i am actually not convinced to move the output column to clob type.
What is the difference in content between OUTPUT and LONG_OUTPUT? I assumed up to now Long_output holds already the full check output, output is only a preview/subset of the same. Varchar2 columns can be extended up to 4k.

Handling of clobs is much more difficult than normal varchar, there are a lot of limitations(no index without add. options etc) and we have already long_output as clob.
Migration would be similar we had with V1.5. Will take much time and space and requires the same changes in code again. Fortunally, i have already a function for insert/update in place. If you really want this, i can change output to clob this week (there is a day off this week in our region on Thu), but pls check with the web gui devs first, if they can handle this type with their php frameworks

I know, after every release the same question:what is the right base to merge/rebase my branches and howto clean the current dirty one?

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 28, 2012

Updated by mfriedrich on 2012-10-28 14:53:14 +00:00

Tommi wrote:

As stated before, i am actually not convinced to move the output column to clob type.
What is the difference in content between OUTPUT and LONG_OUTPUT? I assumed up to now Long_output holds already the full check output, output is only a preview/subset of the same. Varchar2 columns can be extended up to 4k.

that's described in detail in the plugin api - http://docs.icinga.org/latest/en/pluginapi.html#outputspec

long_output will only populated if your plugin actually returns a second (or more lines). the first line is considered output only, so both don't have anything in common - only the order. perfdata is fully optional, and therefore it got a different delimiter (the pipe).

Handling of clobs is much more difficult than normal varchar, there are a lot of limitations(no index without add. options etc) and we have already long_output as clob.
Migration would be similar we had with V1.5. Will take much time and space and requires the same changes in code again. Fortunally, i have already a function for insert/update in place. If you really want this, i can change output to clob this week (there is a day off this week in our region on Thu), but pls check with the web gui devs first, if they can handle this type with their php frameworks

i guess, icinga-web can handle that properly, as it shows the perfdata field in the status details.

the output restrictions only apply to the full string, so you might get even 8k of output http://docs.icinga.org/latest/en/pluginapi.html#outputlengthrestrictions and there are some ppl having recompiled the core for even 16k output length.

so the clob handling such data would make sense to me, instead of cutting it.

I know, after every release the same question:what is the right base to merge/rebase my branches and howto clean the current dirty one?

'dev/.*'
if you cannot see required changes there, ask to merge from 'next'. but not the release branches, nor master. they contain different versions, dates, settings which would hinder clean development.

how to clean? backup/cherry-pick needed changes, and drop the dirty branch locally and remote, create a new branch sourced from a clean branch (dev(ido e.g.) and cherry-pick the commits saved on your backup branch. since you already poisoned your current branch with the r1.8 commits, rebase or reverts do not make much sense.

updated the developer guidelines a bit, to clarify how to merge.
https://wiki.icinga.org/display/Dev/Developer+Guidelines#DeveloperGuidelines-DevBranches

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 29, 2012

Updated by mfriedrich on 2012-10-29 18:39:49 +00:00

  • Status changed from Feedback to 7
  • Target Version set to 1.8.2
  • Done % changed from 90 to 100

looks valid, therefore now merged from your now clean (thanks!) branch into dev/ido. will cherry-pick/merge that later into test and next, as well as r1.8 when applicably tested ok.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 29, 2012

Updated by Tommi on 2012-10-29 20:15:55 +00:00

great.
I am already working on the requested typ change to clob for the output column in all tables on top of this patch. Hopefully i get it running this week. Sql files are prepared, next are the db.c and dbhandlers.c changes. Which version strings should i use for oracle-update-.sql and IDO_SCHEMA_VERSION + IDO_VERSION in common.h? With the upcomming change ido2db schema and code becomes incompatible with the current 1.8.0 implementation

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 29, 2012

Updated by mfriedrich on 2012-10-29 20:22:06 +00:00

this is a behaviourial change, not a fix. i would propose opening a new issue for that, as well as working on a seperated branch, adding all the changes required, as well as the sql upgrade scripts for 1.9. imho that should be tested on the long run, and not for a quickfix release which does not get a beta. so schedule that bigger change in the new issue for 1.9, please.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Oct 29, 2012

Updated by Tommi on 2012-10-29 20:43:55 +00:00

continued in #3412.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Nov 10, 2012

Updated by mfriedrich on 2012-11-10 15:02:48 +00:00

  • Status changed from 7 to Resolved

works for me, thanks.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Dec 8, 2014

Updated by mfriedrich on 2014-12-08 14:37:54 +00:00

  • Project changed from 18 to Core, Classic UI, IDOUtils
  • Category changed from 79 to IDOUtils
  • Icinga Version changed from 1 to 1
  • OS Version set to any

@icinga-migration icinga-migration added this to the 1.8.2 milestone Jan 17, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.