You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/10/19 19:55:00 UTC
[GitHub] [skywalking-php] phanalpha opened a new pull request, #24: blocking (channel) sender and PDO exceptions.
phanalpha opened a new pull request, #24:
URL: https://github.com/apache/skywalking-php/pull/24
A non-blocking sender fails with `EAGAIN`. And a `call_user_function` (`zend_call_function`) fails when exception is set.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1285311153
> We know there's indeed a bug. What have you done with the checks? @jmjoy
Maybe the number of endpoints has increased, so the bug of MacOS has emerged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000177726
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
At first, I used `UnixDatagram`, but I switched to `UnixStream` due to problems with MacOS.
`UnixDatagram` can provide atomic send?
https://github.com/apache/skywalking-php/pull/15
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] heyanlong commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
heyanlong commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000068398
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
Should an option be added to `php.ini` instead of writing it as fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000117101
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
> Then why do we need it to be 'cross process'?
Because php-fpm is `multi-process` arch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000116746
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
Then why do we need it to be 'cross process'?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] wu-sheng commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1285464513
> In fact, I don't want to support running on MacOS, It's painful to be compatible with MacOS, I want to just keep the compilation passed.
That is never a hard requirement. APM focuses on service monitoring in the product env, which is impossible to be a Mac.
So, generally, this is only a question we want to provide this for MAC locally testing, whether it is meaningful for PHP developers. You are more professional than me :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1286468650
> Will there be more and more threads?
There will be one extra thread for each fpm. Simply sending to a non-blocking stream (or so) without retries is much too unreliable. If we could prove that it (send and forget) is acceptable on Linux, maybe it's better to apply these on macOS only (with conditional compilation). Nothing comes at no cost.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1286647767
> We know there are bugs on `Reporter` (channel/worker). Have we made our decisions over that?
Well, you can create an issue follow (https://github.com/apache/skywalking-php#how-to-create-issue).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] wu-sheng commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000070132
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
I agree with @jmjoy We should not ever block main process.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000112320
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
I agree with the considerations on the main process. As I mentioned, a non-blocking stream fails on `write_all` occasionally (actually, not that occasional). This method (`write_all`) will continuously call `write`, which is not *atomic*. Since we are using a prefix length to serialize and deserialize the data, a `write_all` failure would be unrecoverable (from the stream). IMHO, that is not acceptable either.
A blocking `sender` is not good, but an option does not help. Why do we communicate via a hand-crafted Unix pipe instead of a Rust native channel?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1285261082
> A non-blocking sender simply fails macos tests. Any ideas?
Try again. :sweat_smile:
In fact, I don't want to support running on MacOS, It's painful to be compatible with MacOS, I want to just keep the compilation passed.@heyanlong @wu-sheng
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000129167
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
Would the problem be simpler if we utilize one worker for each fpm first? Anyway, the telemetry data are multiplexed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000120697
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
> If we could not ensure the packet is atomic, recovering from a broken stream would be a challenge.
Yes, but blocking is more undesirable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000354941
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
Shall we create an issue/task for it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1287594635
@phanalpha Issue has created. https://github.com/apache/skywalking/issues/9831
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000055450
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
The design principle of skywalking agent should be to ensure that the main
programs are not affected.
If blocking, when the unix stream buffer is full or the grpc process hangs, blocking will cause the php worker process to block.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000129167
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
Would the problem be simpler if we utilize one worker for each fpm first? Anyway, the telemetry data are multiplexed.
A probe is expected to have minor impacts on performance. But still, too many failures could cause a biased observation. Both performance and predictability are critical. It should either work (with acceptable fail rates) or die as a whole.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000113667
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
Rust native channel means `mpsc`? It can't be used cross process, I think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000234845
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
We should not rely on any OS non-blocking methods directly. These operations are somewhat out of control. A buffer could be applied to coordinate the stream, of which the size (or so) is configurable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1286484506
> > However, can you also solve the problem of CI by just setting nonlocking to false?
>
> By rolling back to [9088251](https://github.com/apache/skywalking-php/commit/90882519a2a59bbbec1efd2e33ad0a1a52ca08e1)?
Yes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000187237
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
IMHO, `UnixDatagram` is a better choice here. A `Datagram` is either sent or not. At least the failure is localized, which simplifies the logic. A blocking sender does not actually solve the problem but makes the behavior predictable. I think we could fix it in another PR. We are actually implementing a local proxy to the backend. I'm not quite sure whether it is a good design.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1285234091
A non-blocking sender simply fails macos tests. Any ideas?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000116344
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
The problem of `write` is not atomic, I have an idea, disassemble the data into separate packets and assemble them at the receiving end, similar to multiplexing. I will solve this problem in the next PR.
So now keep `nonlocking` true?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1286057879
It should be able to pass all the tests for now, with a more predictable blocking sender but in another standalone thread per fpm. The non-atomic `write_all` should be solved in another PR.
Though macOS is not our primary target, it does help reveal the bugs. It just *should* work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000055450
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
The design principle of skywalking agent should be to ensure that the main
programs are not affected. If blocking, the unix stream buffer is full or the grpc process hangs, blocking will cause the php worker process to block.
##########
src/exception_frame.rs:
##########
@@ -0,0 +1,38 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use phper::sys;
+
+pub struct ExceptionFrame {
+ #[allow(dead_code)]
+ not_matter: i32,
Review Comment:
This code is not necessary, so remove it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy merged pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy merged PR #24:
URL: https://github.com/apache/skywalking-php/pull/24
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1286330471
> It should be able to pass all the tests for now, with a more predictable blocking sender but in another standalone thread per fpm. The non-atomic `write_all` should be solved in another PR. Though macOS is not our primary target, it does help reveal the bugs. It just _should_ work.
Great job!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1286335124
However, can you also solve the problem of CI by setting nonlocking to false?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000118535
##########
src/exception_frame.rs:
##########
@@ -0,0 +1,38 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use phper::sys;
+
+pub struct ExceptionFrame {
+ #[allow(dead_code)]
+ not_matter: i32,
Review Comment:
Maybe you can use [PhantomData](https://doc.rust-lang.org/std/marker/struct.PhantomData.html) here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000114654
##########
src/exception_frame.rs:
##########
@@ -0,0 +1,38 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use phper::sys;
+
+pub struct ExceptionFrame {
+ #[allow(dead_code)]
+ not_matter: i32,
Review Comment:
I'm not that familiar with Rust patterns. Since we are applying an [RAII](https://en.wikipedia.org/wiki/Resource_acquisition_is_initialization) design here, the dead code could prevent a client from accidentally bypassing the `new` operation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000118164
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
> The problem of `write` is not atomic, I have an idea, disassemble the data into separate packets and assemble them at the receiving end, similar to multiplexing. I will solve this problem in the next PR.
>
> So now keep `nonlocking` true?
If we could not ensure the packet is atomic, recovering from a broken stream would be a challenge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] jmjoy commented on a diff in pull request #24: blocking (channel) sender and PDO exceptions.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on code in PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#discussion_r1000259065
##########
src/channel.rs:
##########
@@ -33,7 +33,7 @@ static RECEIVER: OnceCell<Mutex<Option<StdUnixStream>>> = OnceCell::new();
pub fn init_channel() -> anyhow::Result<()> {
let (sender, receiver) = StdUnixStream::pair()?;
- sender.set_nonblocking(true)?;
+ sender.set_nonblocking(false)?;
Review Comment:
OK, let's rollback this code, and new a PR to deal with it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1285287822
We know there's indeed a bug. What have you done with the checks? @jmjoy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1286473146
> However, can you also solve the problem of CI by just setting nonlocking to false?
By rolling back to [9088251](https://github.com/apache/skywalking-php/commit/90882519a2a59bbbec1efd2e33ad0a1a52ca08e1)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-php] phanalpha commented on pull request #24: Save PDO exceptions.
Posted by GitBox <gi...@apache.org>.
phanalpha commented on PR #24:
URL: https://github.com/apache/skywalking-php/pull/24#issuecomment-1286585181
We know there are bugs on `Reporter` (channel/worker). Have we made our decisions over that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org